[PATCH] bootstd: Fix a handful of doc typos in bootmeth
Fix some trivial typos found by browsing the code.
Done with flyspell.
Signed-off-by: Mattijs Korpershoek
---
include/bootmeth.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/bootmeth.h b/include/bootmeth.h
index 0fc36104ece0..529c4d813d82 100644
--- a/include/bootmeth.h
+++ b/include/bootmeth.h
@@ -40,7 +40,7 @@ struct bootmeth_ops {
/**
* get_state_desc() - get detailed state information
*
-* Prodecues a textual description of the state of the bootmeth. This
+* Produces a textual description of the state of the bootmeth. This
* can include newline characters if it extends to multiple lines. It
* must be a nul-terminated string.
*
@@ -138,7 +138,7 @@ struct bootmeth_ops {
* @dev:Bootmethod device to boot
* @bflow: Bootflow to boot
* Return: does not return on success, since it should boot the
-* Operating Systemn. Returns -EFAULT if that fails, -ENOTSUPP if
+* Operating System. Returns -EFAULT if that fails, -ENOTSUPP if
* trying method resulted in finding out that is not actually
* supported for this boot and should not be tried again unless
* something changes, other -ve on other error
@@ -151,7 +151,7 @@ struct bootmeth_ops {
/**
* bootmeth_get_state_desc() - get detailed state information
*
- * Prodecues a textual description of the state of the bootmeth. This
+ * Produces a textual description of the state of the bootmeth. This
* can include newline characters if it extends to multiple lines. It
* must be a nul-terminated string.
*
@@ -244,7 +244,7 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow
*bflow,
* @dev: Bootmethod device to use
* @bflow: Bootflow to read
* Return: does not return on success, since it should boot the
- * Operating Systemn. Returns -EFAULT if that fails, other -ve on
+ * Operating System. Returns -EFAULT if that fails, other -ve on
* other error
*/
int bootmeth_read_all(struct udevice *dev, struct bootflow *bflow);
@@ -255,7 +255,7 @@ int bootmeth_read_all(struct udevice *dev, struct bootflow
*bflow);
* @dev: Bootmethod device to boot
* @bflow: Bootflow to boot
* Return: does not return on success, since it should boot the
- * Operating Systemn. Returns -EFAULT if that fails, other -ve on
+ * Operating System. Returns -EFAULT if that fails, other -ve on
* other error
*/
int bootmeth_boot(struct udevice *dev, struct bootflow *bflow);
@@ -264,7 +264,7 @@ int bootmeth_boot(struct udevice *dev, struct bootflow
*bflow);
* bootmeth_setup_iter_order() - Set up the ordering of bootmeths to scan
*
* This sets up the ordering information in @iter, based on the selected
- * ordering of the bootmethds in bootstd_priv->bootmeth_order. If there is no
+ * ordering of the bootmeths in bootstd_priv->bootmeth_order. If there is no
* ordering there, then all bootmethods are added
*
* @iter: Iterator to update with the order
---
base-commit: ea722aa5eb33740ae77e8816aeb72b385e621cd0
change-id: 20240603-bootmeth-typos-47c865e70ccf
Best regards,
--
Mattijs Korpershoek
[PATCH] cmd: bcb: Fix bcb compilation when CONFIG_CMD_BCB=n
commit dfeb4f0d7935 ("cmd: bcb: extend BCB C API to allow read/write the
fields")
introduced the bcb_get() function.
When CONFIG_CMD_BCB=n, that function is stubbed.
The stubbed function has a wrong prototype: value_size arg is missing.
Add the missing argument to fix build when CONFIG_CMD_BCB=n.
Fixes: dfeb4f0d7935 ("cmd: bcb: extend BCB C API to allow read/write the
fields")
Signed-off-by: Mattijs Korpershoek
---
include/bcb.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/bcb.h b/include/bcb.h
index 1941d8c28b4f..a56b547595a6 100644
--- a/include/bcb.h
+++ b/include/bcb.h
@@ -58,7 +58,8 @@ static inline int bcb_set(enum bcb_field field, const char
*value)
return -EOPNOTSUPP;
}
-static inline int bcb_get(enum bcb_field field, char *value_out)
+static inline int bcb_get(enum bcb_field field,
+ char *value_out, size_t value_size)
{
return -EOPNOTSUPP;
}
---
base-commit: ea722aa5eb33740ae77e8816aeb72b385e621cd0
change-id: 20240603-bcb-compil-d8eaf7074475
Best regards,
--
Mattijs Korpershoek
Re: [PATCH v5 02/23] doc: ti: k3: Correct spelling mistakes and improve clarity
Hi Jon, Thank you for the patch. On ven., mai 31, 2024 at 17:20, Jonathan Humphreys wrote: > Few cosmetic fixes for clarity and spelling mistakes. > > Signed-off-by: Jonathan Humphreys Reviewed-by: Mattijs Korpershoek > --- > doc/board/ti/k3.rst | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/doc/board/ti/k3.rst b/doc/board/ti/k3.rst > index a1c01d1cf02..927f3976d34 100644 > --- a/doc/board/ti/k3.rst > +++ b/doc/board/ti/k3.rst > @@ -51,14 +51,14 @@ For all K3 SoCs the first core started will be inside the > Security > Management Subsystem (SMS) which will secure the device and start a core > in the wakeup domain to run the ROM code. ROM will then initialize the > boot media needed to load the binaries packaged inside `tiboot3.bin`, > -including a 32bit U-Boot SPL, (called the wakup SPL) that ROM will jump > +including a 32bit U-Boot SPL, (called the wakeup SPL) that ROM will jump > to after it has finished loading everything into internal SRAM. > > .. image:: img/boot_flow_01.svg >:alt: Boot flow up to wakeup domain SPL > > The wakeup SPL, running on a wakeup domain core, will initialize DDR and > -any peripherals needed load the larger binaries inside the `tispl.bin` > +any peripherals needed to load the larger binaries inside the `tispl.bin` > into DDR. Once loaded the wakeup SPL will start one of the 'big' > application cores inside the main domain to initialize the main domain, > starting with Trusted Firmware-A (TF-A), before moving on to start > @@ -94,7 +94,7 @@ essentially 4 unique but very similar flows: > * Combined binary with a split firmware: (eg: AM62) > > For devices that utilize the split binary approach, ROM is not capable > -of loading the firmware into the SoC requiring the wakeup domain's > +of loading the firmware into the SoC, requiring the wakeup domain's > U-Boot SPL to load the firmware. > > Devices with a split firmware will have two firmwares loaded into the > @@ -114,8 +114,8 @@ K3 HS-SE (High Security - Security Enforced) devices > enforce an > authenticated boot flow for secure boot. HS-FS (High Security - Field > Securable) is the state of a K3 device before it has been eFused with > customer security keys. In the HS-FS state the authentication still can > -function as in HS-SE but as there are no customer keys to verify the > -signatures against the authentication will pass for certificates signed > +function as in HS-SE, but as there are no customer keys to verify the > +signatures against, the authentication will pass for certificates signed > with any key. > > Chain of trust > -- > 2.34.1
Re: [PATCH v2 0/8] Add DFU, emmc and usb boot for TI am62x
Hi Dhruva, On mer., mai 29, 2024 at 11:39, Dhruva Gole wrote: > Hi Sjoerd, > > On Aug 21, 2023 at 14:20:26 -0400, Tom Rini wrote: >> On Mon, Aug 21, 2023 at 04:13:32PM +, Marcel Ziswiler wrote: >> > Hi Sjoerd >> > >> > On Thu, 2023-06-01 at 08:37 +0200, Sjoerd Simons wrote: >> > > On Wed, 2023-05-31 at 17:14 -0400, Tom Rini wrote: >> > > > On Thu, Apr 06, 2023 at 08:55:34PM +0200, Sjoerd Simons wrote: >> > > > >> > > > > This series adds more boot sources for the TI am62x. For that the >> > > > > dts' >> > > > > are synced from the upstream ti-next git tree (to add usb nodes), >> > > > > some >> > > > > dwc3 glue is and finally the default configuration is tuned to add >> > > > > support for DFU and USB (host and gadget) >> > > > >> > > > This seems, conceptually, fine. But as we're getting the TI dts >> > > > files >> > > > in sync with the kernel, I'm deferring this version and you'll want >> > > > to >> > > > rebase and re-post once everything has settled. >> > > >> > > Thanks for the update/hint ;) I also got a few review comments so the >> > > plan is to include those and repost.. Just my may has been stupidly >> > > busy causing me to not get around it in the first place, so maybe that >> > > turned into good timing in the end. >> > >> > Any progress on this? >> > >> > I still carry your re-based series on top of latest master [1] and USB DFU >> > is working very well on Verdin AM62. >> > >> > Thanks! >> > >> > [1] https://github.com/ziswiler/u-boot/tree/verdin-am62-usb-support >> >> As the am62 files have been re-synced, re-basing this and re-posting for >> next would be appropriate at this point. > > Any updates on this? Do you have plans to rebase and resend this series? A part of this (USB DFU) has been taken over by Martyn and has been merged: https://lore.kernel.org/all/171581573300.812585.12291710364879103682.b4...@konsulko.com/ > > > > -- > Best regards, > Dhruva Gole
Re: [PATCH v2 1/8] dfu: add scsi backend
Hi Caleb, Thank you for the patch. On lun., mai 27, 2024 at 19:17, Caleb Connolly wrote: > This is extremely similar to the MMC backend, but there are some notable > differences. > > Works with a DFU string like > > scsi 4=u-boot-bin part 11 > > Where "4" is the SCSI dev number (sequential LUN across all SCSI devices) > and "11" is the partition number. > > Signed-off-by: Caleb Connolly > --- > doc/usage/dfu.rst | 31 > drivers/dfu/Kconfig| 7 + > drivers/dfu/Makefile | 1 + > drivers/dfu/dfu.c | 5 +- > drivers/dfu/dfu_scsi.c | 437 > + > include/dfu.h | 26 +++ > 6 files changed, 506 insertions(+), 1 deletion(-) I reviewed this here [1]. I could not find any response on that thread. Could you please respond there or address the remarks for v3, please? [1] https://lore.kernel.org/r/87o7a94pe1@baylibre.com > > diff --git a/doc/usage/dfu.rst b/doc/usage/dfu.rst > index 8cc09c308d82..dc4f8d672f99 100644 > --- a/doc/usage/dfu.rst > +++ b/doc/usage/dfu.rst > @@ -166,8 +166,38 @@ mmc [...] > -- > 2.45.0
Re: [PATCH] usb: dwc3: gadget: fix crash in dwc3_gadget_giveback()
Hi Neil,
Thank you for the patch.
On mar., mai 28, 2024 at 10:35, Neil Armstrong
wrote:
> If the ep0 stalls or request are dequeued when gagdet is stopped,
> the request dma may not be mapped yet and dwc3_flush_cache() may be
> called with a NULL pointer.
>
> Check req->request.dma before calling dwc3_flush_cache() and later
> the usb_gadget_unmap_request() functions since it means that
> usb_gadget_map_request() hasn't been called yet.
>
> Fixes: fd15b58c1a9 ("dwc3: flush cache only if there is a buffer attached to
> a request")
> Signed-off-by: Neil Armstrong
Reviewed-by: Mattijs Korpershoek
> ---
> drivers/usb/dwc3/gadget.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index fab32575647..92c7c6d08b7 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -248,7 +248,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct
> dwc3_request *req,
>
> list_del(>list);
> req->trb = NULL;
> - if (req->request.length)
> + if (req->request.dma && req->request.length)
> dwc3_flush_cache((uintptr_t)req->request.dma,
> req->request.length);
>
> if (req->request.status == -EINPROGRESS)
> @@ -256,7 +256,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct
> dwc3_request *req,
>
> if (dwc->ep0_bounced && dep->number == 0)
> dwc->ep0_bounced = false;
> - else
> + else if (req->request.dma)
> usb_gadget_unmap_request(>gadget, >request,
> req->direction);
>
>
> ---
> base-commit: 7e52d6ccfb76e2afc2d183b357abe2a2e2f948cf
> change-id: 20240528-topic-sm8x50-dwc3-gadget-crash-fix-fa0404ffce33
>
> Best regards,
> --
> Neil Armstrong
Re: [PATCH 3/3] abootimg: Implement smart image load feature
Hi Roman,
Thank you for the patch.
On dim., mai 19, 2024 at 19:18, Roman Stratiienko
wrote:
> Load only part of the boot partition that contains valuable
> information, thus improving the boot time.
>
> Signed-off-by: Roman Stratiienko
> ---
> boot/image-android.c | 70
> cmd/abootimg.c | 40 ++---
> include/image.h | 12
> 3 files changed, 118 insertions(+), 4 deletions(-)
>
> diff --git a/boot/image-android.c b/boot/image-android.c
> index da8003f370..d00a896a40 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -204,6 +204,76 @@ bool android_image_get_data(const void *boot_hdr, const
> void *vendor_boot_hdr,
> return true;
> }
>
> +/**
> + * android_image_get_valuable_size() - get the size of the android image
> + *
> + * This function checks if the image is Android boot image and returns the
> + * valuable size of the image.
> + *
> + * @hdr_addr: Boot image header address (boot or vendor_boot)
> + *
> + * @return size of the image on success, 0 on failure
> + */
> +size_t android_image_get_valuable_size(const void *hdr_addr)
Can't we use a ssize_t to return a negative error code?
0 as error seems odd and not what is used in most functions in
boot/image-android.c
> +{
> + int version, size;
> +
> + if (is_android_boot_image_header(hdr_addr)) {
> + const struct andr_boot_img_hdr_v0 *hdr = hdr_addr;
> +
> + version = ((struct andr_boot_img_hdr_v0
> *)hdr_addr)->header_version;
> + if (version > 2) {
> + const struct andr_boot_img_hdr_v3 *hdr = hdr_addr;
> +
> + size = ALIGN(hdr->header_size, ANDR_GKI_PAGE_SIZE);
> + size += ALIGN(hdr->kernel_size, ANDR_GKI_PAGE_SIZE);
> + size += ALIGN(hdr->ramdisk_size, ANDR_GKI_PAGE_SIZE);
> +
> + if (version > 3)
> + size += ALIGN(hdr->signature_size,
> ANDR_GKI_PAGE_SIZE);
> +
> + return size;
> + }
> +
> + size = hdr->page_size;
> + size += ALIGN(hdr->kernel_size, hdr->page_size);
> + size += ALIGN(hdr->ramdisk_size, hdr->page_size);
> + size += ALIGN(hdr->second_size, hdr->page_size);
> +
> + if (version > 0)
> + size += ALIGN(hdr->recovery_dtbo_size, hdr->page_size);
> +
> + if (version > 1)
> + size += ALIGN(hdr->dtb_size, hdr->page_size);
> +
> + return size;
> + }
> +
> + if (is_android_vendor_boot_image_header(hdr_addr)) {
> + const struct andr_vnd_boot_img_hdr *hdr = hdr_addr;
> +
> + version = ((struct andr_vnd_boot_img_hdr
> *)hdr_addr)->header_version;
> + if (version < 3) {
> + printf("Vendor boot image header version %d is not
> supported\n", version);
> +
> + return 0;
> + }
> +
> + size = ALIGN(hdr->header_size, hdr->page_size);
> + size += ALIGN(hdr->vendor_ramdisk_size, hdr->page_size);
> + size += ALIGN(hdr->dtb_size, hdr->page_size);
> +
> + if (version > 3) {
> + size += ALIGN(hdr->vendor_ramdisk_table_size,
> hdr->page_size);
> + size += ALIGN(hdr->bootconfig_size, hdr->page_size);
> + }
> +
> + return size;
> + }
> +
> + return 0;
> +}
> +
> static ulong android_image_get_kernel_addr(struct andr_image_data *img_data)
> {
> /*
> diff --git a/cmd/abootimg.c b/cmd/abootimg.c
> index 808c9c4941..fe7c5c5e2c 100644
> --- a/cmd/abootimg.c
> +++ b/cmd/abootimg.c
> @@ -182,6 +182,35 @@ static int abootimg_get_dtb(int argc, char *const argv[])
> return CMD_RET_USAGE;
> }
>
> +static int abootimg_smart_load(struct blk_desc *desc, struct disk_partition
> *info, void *addr)
> +{
> + int ret, size;
> +
> + ret = blk_dread(desc, info->start, 4096 / info->blksz, addr);
Why 4096? Can we use #define for this or reuse an exising definition?
> + if (ret < 0) {
> + printf("Error: Failed to read partition\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + size = android_image_get_valuable_size(addr);
> + if (size == 0)
> + return 0;
> +
> + ret = blk_dread(desc, info->start, DIV_ROUND_UP(size, info->blksz),
> addr);
> + if (ret < 0) {
> + printf("Error: Failed to read partition\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + memset(addr + size, 0, info->size * info->blksz - size);
> +
> + printf("Loaded Android boot image using smart load (%d/%d MB)\n",
Why only boot image? Should this be more generic be passing the
partition name instead?
> +(int)DIV_ROUND_UP(size, 1024 * 1024),
> +(int)DIV_ROUND_UP(info->size * info->blksz, 1024 * 1024));
> +
> + return
Re: [PATCH 2/3] avb: Implement get_preloaded_partition callback
Hi Roman,
Thank you for the patch.
On dim., mai 19, 2024 at 19:18, Roman Stratiienko
wrote:
> AVB can reuse already loaded images instead of loading them
> from the disk.
>
> The get_preloaded_partition now looks for the env. variables
> set by the 'abootimg load' to find the correct partition in RAM.
>
> Signed-off-by: Roman Stratiienko
> ---
> common/avb_verify.c | 53 +
> 1 file changed, 53 insertions(+)
>
> diff --git a/common/avb_verify.c b/common/avb_verify.c
> index cff9117d92..d2626e8844 100644
> --- a/common/avb_verify.c
> +++ b/common/avb_verify.c
> @@ -6,6 +6,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -595,6 +596,55 @@ static AvbIOResult read_from_partition(AvbOps *ops,
> num_bytes, buffer, out_num_read, IO_READ);
> }
>
> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
> +/**
> + * get_preloaded_partition() - Gets the starting pointer of a partition that
> + * is pre-loaded in memory, and save it to |out_pointer|.
> + *
> + * If the partition is not pre-loaded in memory, the out_pointer shall not be
> + * modified.
> + *
> + * @ops: contains AVB ops handlers
> + * @partition: partition name, NUL-terminated UTF-8 string
NUL -> NULL
> + * @num_bytes: amount of bytes to read
> + * @out_pointer: pointer to the starting address of the partition
> + * @out_num_bytes_preloaded: amount of bytes pre-loaded in memory
> + *
> + * @return:
> + * AVB_IO_RESULT_OK, if partition was found or was not found
Add:
AVB_IO_RESULT_ERROR_IO, if partition size is smaller than requested
With both small remarks addressed, please add:
Reviewed-by: Mattijs Korpershoek
> + *
> + */
> +static AvbIOResult get_preloaded_partition(AvbOps *ops, const char
> *partition, size_t num_bytes,
> +uint8_t **out_pointer, size_t
> *out_num_bytes_preloaded)
> +{
[...]
> --
> 2.40.1
Re: [PATCH 1/3] abootcmd: Add load subcommand
Hi Roman,
Thank you for the patch.
On dim., mai 19, 2024 at 19:18, Roman Stratiienko
wrote:
> What it does:
> 1. Allocates the memory in HEAP to fit the partition
> 2. Loads partition into memory. In the following patch of this series,
>loading will be optimized to avoid loading an empty space.
> 3. Sets buffer start and buffer size value into environment variables
>abootimg__ptr and abootimg__size, respectively.
> and duplicate them as
>abootimg___ptr and abootimg___size.
>The latter two are needed to access by the AVB get_preloaded_partition.
>(see the next patch).
>
> Before this command, the boot script developer was responsible for
> allocating the memory manually by choosing the start and the end,
> which is far from good.
>
> Usage example:
>
> abootcmd load mmc 0 boot a
Should this be: abootimg load mmc 0 boot a ?
>
> Signed-off-by: Roman Stratiienko
> ---
> cmd/abootimg.c | 84 +-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/abootimg.c b/cmd/abootimg.c
> index a5321bab6a..808c9c4941 100644
> --- a/cmd/abootimg.c
> +++ b/cmd/abootimg.c
> @@ -8,7 +8,9 @@
> #include
> #include
> #include
> +#include
> #include
> +#include
>
> #define abootimg_addr() \
> (_abootimg_addr == -1 ? image_load_addr : _abootimg_addr)
> @@ -259,10 +261,81 @@ static int do_abootimg_dump(struct cmd_tbl *cmdtp, int
> flag, int argc,
> return CMD_RET_SUCCESS;
> }
>
> +static int do_abootimg_load(struct cmd_tbl *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + int time_start = get_timer(0);
On -next branch (where the patch will be applied), I see the following
build error:
cmd/abootimg.c: In function ‘do_abootimg_load’:
cmd/abootimg.c:279:26: error: implicit declaration of function ‘get_timer’
[-Wimplicit-function-declaration]
279 | int time_start = get_timer(0);
| ^
Please add the missing include: #include
Base: 7e52d6ccfb76 ("Merge patch series "FWU: Add support for FWU metadata
version 2"")
> + struct blk_desc *desc;
> + struct disk_partition info;
> + char buf[512] = { 0 };
> + void *addr;
> + int ret;
> +
> + if (argc < 4)
> + return CMD_RET_USAGE;
> + if (argc > 5)
> + return CMD_RET_USAGE;
Can we please do this in a single line, like done in other commands such
as do_abootimg_addr()?
> +
> + ret = blk_get_device_by_str(argv[1], argv[2], );
> + if (ret < 0) {
> + printf("Error: Failed to get device %s %s\n", argv[1], argv[2]);
> + return CMD_RET_FAILURE;
> + }
> +
> + if (argc == 5)
> + sprintf(buf, "%s_%s", argv[3], argv[4]);
> + else
> + sprintf(buf, "%s", argv[3]);
> +
> + ret = part_get_info_by_name(desc, buf, );
> + if (ret < 0) {
> + printf("Error: Failed to get partition %s\n", buf);
> + return CMD_RET_FAILURE;
> + }
> +
> + addr = (void *)memalign(4096, info.size * info.blksz);
Why 4096? If this is a known number, can we use a define for it please?
> + if (!addr) {
> + printf("Error: Failed to allocate memory\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + ret = blk_dread(desc, info.start, info.size, addr);
> + if (ret < 0) {
> + printf("Error: Failed to read partition %s\n", buf);
> + goto fail;
> + }
> +
> + sprintf(buf, "abootimg_%s_ptr", argv[3]);
> + env_set_hex(buf, (ulong)addr);
> +
> + sprintf(buf, "abootimg_%s_size", argv[3]);
> + env_set_hex(buf, info.size * info.blksz);
> +
> + if (argc == 5) {
> + sprintf(buf, "abootimg_%s_%s_ptr", argv[3], argv[4]);
> + env_set_hex(buf, (ulong)addr);
> +
> + sprintf(buf, "abootimg_%s_%s_size", argv[3], argv[4]);
> + env_set_hex(buf, info.size * info.blksz);
> + }
> +
> + int time_end = get_timer(0);
> +
> + printf("Loaded '%s' partition to address 0x%p (size: 0x%x) in %lu ms\n",
> +argv[3], addr, info.size * info.blksz, time_end - time_start);
This causes warnings on -next when building sandbox:
$ make sandbox_defconfig
$ make
cmd/abootimg.c:339:65: warning: format ‘%x’ expects argument of type ‘unsigned
int’, but argument 4 has type ‘long unsigned int’ [-Wformat=]
339 | printf("Loaded '%s' partition to address 0x%p (size: 0x%x) in
%lu ms\n",
|~^
| |
|
unsigned int
|%lx
340 |argv[3], addr, info.size * info.blksz, time_end -
time_start);
| ~~
|
[PATCH] doc: cmd: bootmeth: Fix extlinunx -> extlinux typo
Fix a trivial typo in the bootmeth documentation. Signed-off-by: Mattijs Korpershoek --- doc/usage/cmd/bootmeth.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/usage/cmd/bootmeth.rst b/doc/usage/cmd/bootmeth.rst index 2903977ee54d..bac9fdf85cdc 100644 --- a/doc/usage/cmd/bootmeth.rst +++ b/doc/usage/cmd/bootmeth.rst @@ -48,7 +48,7 @@ The format looks like this: = === == = Order Seq NameDescription = === == = -00 extlinunx Extlinux boot from a block device +00 extlinuxExtlinux boot from a block device 11 efi EFI boot from an .efi file 22 pxe PXE boot from a network device 33 sandbox Sandbox boot for testing --- base-commit: a7f0154c412859323396111dd0c09dbafbc153cb change-id: 20240524-bootflow-doc-typo-9339f86cf340 Best regards, -- Mattijs Korpershoek
[PATCH] image: Set load_end on partial loads
When decompressing, it's possible that the algorithm only performs
a partial decompression.
This usually happens when CONFIG_SYS_BOOTM_LEN is too small for
the uncompressed image.
When that happens, image_decomp() returns an error and *load_end == load.
The error is then handled by handle_decomp_error().
handle_decomp_error() expects the number of uncompressed bytes in
uncomp_size but receives *load_end - load == load - load == 0.
Because of this, handle_decomp_error does not report the expected
"Image too large: increase CONFIG_SYS_BOOTM_LEN" error message.
Modify the image_decomp() logic to always report the decompressed size,
even when a partial decompression happened.
Signed-off-by: Mattijs Korpershoek
---
This has been tested on an AM62X SK EVM board with a big lz4 image and
the default CONFIG_SYS_BOOTM_LEN of 0x80.
This has also been tested on sandbox using:
=> ut compression
---
boot/image.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/boot/image.c b/boot/image.c
index 073931cd7a3f..4f48e6eb563d 100644
--- a/boot/image.c
+++ b/boot/image.c
@@ -531,10 +531,10 @@ int image_decomp(int comp, ulong load, ulong image_start,
int type,
printf("Unimplemented compression type %d\n", comp);
return ret;
}
- if (ret)
- return ret;
*load_end = load + image_len;
+ if (ret)
+ return ret;
return 0;
}
---
base-commit: a7f0154c412859323396111dd0c09dbafbc153cb
change-id: 20240523-image-partial-decomp-d6604e998e3a
Best regards,
--
Mattijs Korpershoek
Re: [PATCH v3] abootimg: Add init_boot image support
Hi Roman, Thank you for the patch. On jeu., mai 23, 2024 at 07:06, Roman Stratiienko wrote: > Quote from [1]: > > "For devices launching with Android 13, the generic ramdisk is removed > from the boot image and placed in a separate init_boot image. > This change leaves the boot image with only the GKI kernel." > > While at it, update wrong error handling message when vendor_boot > cannot be loaded. > > [1]: https://source.android.com/docs/core/architecture/partitions/generic-boot > Signed-off-by: Roman Stratiienko > Reviewed-by: Mattijs Korpershoek To confirm, v3 applies without any conflicts. This series was initially assigned to Tom on patchwork, so it's up to him to see when he wants to pick this up. [...]
Re: [PATCH v2] abootimg: Add init_boot image support
Hi Roman,
Thank you for the patch.
On mer., mai 22, 2024 at 21:26, Roman Stratiienko
wrote:
> Quote from [1]:
>
> "For devices launching with Android 13, the generic ramdisk is removed
> from the boot image and placed in a separate init_boot image.
> This change leaves the boot image with only the GKI kernel."
>
> While at it, update wrong error handling message when vendor_boot
> cannot be loaded.
>
> [1]: https://source.android.com/docs/core/architecture/partitions/generic-boot
> Signed-off-by: Roman Stratiienko
Reviewed-by: Mattijs Korpershoek
Note: this patch still does not apply on master nor next:
$ ~/work/upstream/u-boot/ git show --pretty='%h ("%s")' HEAD --no-patch
a7f0154c4128 ("Prepare v2024.07-rc3")
$ ~/work/upstream/u-boot/ b4 shazam -s -l
20240522212645.87250-1-r.stratiie...@gmail.com
[...]
Total patches: 1
---
Applying: abootimg: Add init_boot image support
Patch failed at 0001 abootimg: Add init_boot image support
error: sha1 information is lacking or useless (cmd/abootimg.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
- master: a7f0154c4128 ("Prepare v2024.07-rc3")
- next: 377e91c162ab ("Merge patch series "Clean-up patch set for MbedTLS
integration"")
Looking further down below, we can see that this patch has the "abootimg
load" command, which is introduced in these series:
https://lore.kernel.org/r/20240519191856.2582174-1-r.stratiie...@gmail.com
Please consider rebasing on either master or next before sending.
> ---
> boot/image-board.c | 13 ++---
> cmd/abootimg.c | 26 +-
> include/image.h| 7 +++
> 3 files changed, 38 insertions(+), 8 deletions(-)
[...]
>
>
> static struct cmd_tbl cmd_abootimg_sub[] = {
> - U_BOOT_CMD_MKENT(addr, 3, 1, do_abootimg_addr, "", ""),
> + U_BOOT_CMD_MKENT(addr, 4, 1, do_abootimg_addr, "", ""),
> U_BOOT_CMD_MKENT(dump, 2, 1, do_abootimg_dump, "", ""),
> U_BOOT_CMD_MKENT(get, 5, 1, do_abootimg_get, "", ""),
> U_BOOT_CMD_MKENT(load, 5, 1, do_abootimg_load, "", ""),
> @@ -376,7 +392,7 @@ static int do_abootimg(struct cmd_tbl *cmdtp, int flag,
> int argc,
> U_BOOT_CMD(
> abootimg, CONFIG_SYS_MAXARGS, 0, do_abootimg,
> "manipulate Android Boot Image",
> - "addr []>\n"
> + "addr [ []]\n"
> "- set the address in RAM where boot image is located\n"
> " ($loadaddr is used by default)\n"
> "abootimg dump dtb\n"
[...]
Re: [PATCH] abootimg: Add init_boot image support
Hi Roman,
On jeu., mai 23, 2024 at 00:35, Roman Stratiienko
wrote:
[...]
>> >
>> > static struct cmd_tbl cmd_abootimg_sub[] = {
>> > - U_BOOT_CMD_MKENT(addr, 3, 1, do_abootimg_addr, "", ""),
>> > + U_BOOT_CMD_MKENT(addr, 4, 1, do_abootimg_addr, "", ""),
>> > U_BOOT_CMD_MKENT(dump, 2, 1, do_abootimg_dump, "", ""),
>> > U_BOOT_CMD_MKENT(get, 5, 1, do_abootimg_get, "", ""),
>> > U_BOOT_CMD_MKENT(load, 5, 1, do_abootimg_load, "", ""),
>> > @@ -375,7 +391,7 @@ static int do_abootimg(struct cmd_tbl *cmdtp, int
>> > flag, int argc,
>> > U_BOOT_CMD(
>> > abootimg, CONFIG_SYS_MAXARGS, 0, do_abootimg,
>> > "manipulate Android Boot Image",
>> > - "addr []>\n"
>> > + "addr [
>> > []]\n"
>> > "- set the address in RAM where boot image is located\n"
>> > " ($loadaddr is used by default)\n"
>>
>> Please include the help text update in the documentation:
>> doc/android/boot-image.rst
>
> That documentation does not mention 'abootimg addr' command at the moment.
> I do not see an easy way to add it in a way that makes sense.
>
I see. I will send a documentation update later then.
>>
>> > "abootimg dump dtb\n"
>> > diff --git a/include/image.h b/include/image.h
>> > index be3c73a72e..b1fd6b3149 100644
[...]
Re: [PATCH] abootimg: Add init_boot image support
Hi Roman,
Thank you for the patch.
On dim., mai 19, 2024 at 12:53, Roman Stratiienko
wrote:
> Quote from [1]:
>
> "For devices launching with Android 13, the generic ramdisk is removed
> from the boot image and placed in a separate init_boot image.
> This change leaves the boot image with only the GKI kernel."
>
> [1]: https://source.android.com/docs/core/architecture/partitions/generic-boot
> Signed-off-by: Roman Stratiienko
> ---
> boot/image-board.c | 5 -
> cmd/abootimg.c | 26 +-
> include/image.h| 7 +++
> 3 files changed, 32 insertions(+), 6 deletions(-)
This patch does not apply on:
- next: 7d24c3e06fa9 ("Merge patch series "scripts/setlocalversion sync with
linux 6.9"")
- master: a7f0154c4128 ("Prepare v2024.07-rc3")
Please consider rebasing when resending.
More review below
>
> diff --git a/boot/image-board.c b/boot/image-board.c
> index 75f6906cd5..715654ff01 100644
> --- a/boot/image-board.c
> +++ b/boot/image-board.c
> @@ -414,9 +414,12 @@ static int select_ramdisk(struct bootm_headers *images,
> const char *select, u8 a
> int ret;
> if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
> void *boot_img =
> map_sysmem(get_abootimg_addr(), 0);
> + void *init_boot_img =
> map_sysmem(get_ainit_bootimg_addr(), 0);
> void *vendor_boot_img =
> map_sysmem(get_avendor_bootimg_addr(), 0);
> + void *ramdisk_img = (init_boot_img == -1) ?
> boot_img :
> +
> init_boot_img;
This line introduces a build warning:
../boot/image-board.c: In function ‘select_ramdisk’:
../boot/image-board.c:412:68: warning: comparison between pointer and integer
412 | void *ramdisk_img = (init_boot_img ==
-1) ? boot_img :
|^~
Can we please fix it in v2 ?
>
> - ret = android_image_get_ramdisk(boot_img,
> vendor_boot_img,
> + ret = android_image_get_ramdisk(ramdisk_img,
> vendor_boot_img,
> rd_datap,
> rd_lenp);
> unmap_sysmem(vendor_boot_img);
Can we please add unmap_sysmem(init_boot_img); here as well ?
> unmap_sysmem(boot_img);
> diff --git a/cmd/abootimg.c b/cmd/abootimg.c
> index e68c523705..7c450a3b2d 100644
> --- a/cmd/abootimg.c
> +++ b/cmd/abootimg.c
> @@ -17,6 +17,7 @@
>
> /* Please use abootimg_addr() macro to obtain the boot image address */
> static ulong _abootimg_addr = -1;
> +static ulong _ainit_bootimg_addr = -1;
> static ulong _avendor_bootimg_addr = -1;
>
> ulong get_abootimg_addr(void)
> @@ -24,6 +25,11 @@ ulong get_abootimg_addr(void)
> return (_abootimg_addr == -1 ? image_load_addr : _abootimg_addr);
> }
>
> +ulong get_ainit_bootimg_addr(void)
> +{
> + return _ainit_bootimg_addr;
> +}
> +
> ulong get_avendor_bootimg_addr(void)
> {
> return _avendor_bootimg_addr;
> @@ -209,7 +215,7 @@ static int do_abootimg_addr(struct cmd_tbl *cmdtp, int
> flag, int argc,
> char *endp;
> ulong img_addr;
>
> - if (argc < 2 || argc > 3)
> + if (argc < 2 || argc > 4)
> return CMD_RET_USAGE;
>
> img_addr = hextoul(argv[1], );
> @@ -220,16 +226,26 @@ static int do_abootimg_addr(struct cmd_tbl *cmdtp, int
> flag, int argc,
>
> _abootimg_addr = img_addr;
>
> - if (argc == 3) {
> + if (argc > 2) {
> img_addr = simple_strtoul(argv[2], , 16);
> if (*endp != '\0') {
> - printf("Error: Wrong vendor image address\n");
> + printf("Error: Wrong vendor_boot image address\n");
Nitpick: this seems like a harmless change but could be done in a
separate patch.
To me, it's fine to include this hunk, but can we mention it in the
commit message?
Something along the lines as "While at it, update wrong error handling
message when vendor_boot cannot be loaded".
> return CMD_RET_FAILURE;
> }
>
> _avendor_bootimg_addr = img_addr;
> }
>
> + if (argc == 4) {
> + img_addr = simple_strtoul(argv[3], , 16);
> + if (*endp != '\0') {
> + printf("Error: Wrong init_boot image address\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + _ainit_bootimg_addr = img_addr;
> + }
> +
> return CMD_RET_SUCCESS;
> }
>
> @@ -346,7 +362,7 @@ fail:
> }
>
> static struct cmd_tbl cmd_abootimg_sub[] = {
> - U_BOOT_CMD_MKENT(addr, 3, 1, do_abootimg_addr, "", ""),
> + U_BOOT_CMD_MKENT(addr, 4, 1, do_abootimg_addr, "", ""),
> U_BOOT_CMD_MKENT(dump, 2, 1, do_abootimg_dump, "", ""),
>
Re: [PATCH] android: Fix ramdisk loading for bootimage v3+
Hi Roman,
Thank you for the patch.
I know you reported this problem quite a while ago [1].
Sorry I did not follow up.
On dim., mai 19, 2024 at 13:09, Roman Stratiienko
wrote:
> The boot_ramdisk and vendor_ramdisk must be both concatenated together.
> Without this change, Android root is missing some of the necessary tools
> to complete virtual AB OTA.
>
> Signed-off-by: Roman Stratiienko
Reviewed-by: Mattijs Korpershoek
[1]
https://lore.kernel.org/r/CAGphcdnC124V_D6x06P6apnE+L+PVJSp+88h0FFY449CBgY=p...@mail.gmail.com
> ---
> boot/image-android.c | 17 -
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/boot/image-android.c b/boot/image-android.c
> index 04f2a8015e..499eae9bcd 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -64,7 +64,6 @@ static void android_boot_image_v3_v4_parse_hdr(const struct
> andr_boot_img_hdr_v3
>
> data->kcmdline = hdr->cmdline;
> data->header_version = hdr->header_version;
> - data->ramdisk_ptr = env_get_ulong("ramdisk_addr_r", 16, 0);
>
> /*
>* The header takes a full page, the remaining components are aligned
> @@ -75,6 +74,7 @@ static void android_boot_image_v3_v4_parse_hdr(const struct
> andr_boot_img_hdr_v3
> data->kernel_ptr = end;
> data->kernel_size = hdr->kernel_size;
> end += ALIGN(hdr->kernel_size, ANDR_GKI_PAGE_SIZE);
> + data->ramdisk_ptr = end;
> data->ramdisk_size = hdr->ramdisk_size;
> data->boot_ramdisk_size = hdr->ramdisk_size;
> end += ALIGN(hdr->ramdisk_size, ANDR_GKI_PAGE_SIZE);
> @@ -462,25 +462,24 @@ int android_image_get_ramdisk(const void *hdr, const
> void *vendor_boot_img,
> return -1;
> }
> if (img_data.header_version > 2) {
> - ramdisk_ptr = img_data.ramdisk_ptr;
> + ramdisk_ptr = img_data.ramdisk_addr;
> memcpy((void *)(ramdisk_ptr), (void
> *)img_data.vendor_ramdisk_ptr,
> img_data.vendor_ramdisk_size);
> - memcpy((void *)(ramdisk_ptr + img_data.vendor_ramdisk_size),
> -(void *)img_data.ramdisk_ptr,
> + ramdisk_ptr += img_data.vendor_ramdisk_size;
> + memcpy((void *)(ramdisk_ptr), (void *)img_data.ramdisk_ptr,
> img_data.boot_ramdisk_size);
> + ramdisk_ptr += img_data.boot_ramdisk_size;
> if (img_data.bootconfig_size) {
> memcpy((void *)
> -(ramdisk_ptr + img_data.vendor_ramdisk_size +
> -img_data.boot_ramdisk_size),
> -(void *)img_data.bootconfig_addr,
> +(ramdisk_ptr), (void *)img_data.bootconfig_addr,
> img_data.bootconfig_size);
> }
> }
>
> printf("RAM disk load addr 0x%08lx size %u KiB\n",
> -img_data.ramdisk_ptr, DIV_ROUND_UP(img_data.ramdisk_size, 1024));
> +img_data.ramdisk_addr, DIV_ROUND_UP(img_data.ramdisk_size,
> 1024));
>
> - *rd_data = img_data.ramdisk_ptr;
> + *rd_data = img_data.ramdisk_addr;
>
> *rd_len = img_data.ramdisk_size;
> return 0;
> --
> 2.40.1
[GIT PULL] Please pull u-boot-dfu-20240516
Hi Tom, everyone, Sorry for the double email, I missed to include the list :( Please find some fixes for master: - Fix cdns3 low power hang via fast access bit - Multiple dwc3 gadget fixes, mainly for USB support on TI AM6232 - Consistent USB_GADGET_MANUFACTURER for PHYTEC boards - MAINTAINERS file update for u-boot-dfu The CI job is at https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/20737 Thanks, Mattijs The following changes since commit c8ffd1356d42223cbb8c86280a083cc3c93e6426: Merge patch series "arm: dts: am62-beagleplay: Fix Beagleplay Ethernet" (2024-05-13 09:15:51 -0600) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-dfu.git tags/u-boot-dfu-20240516 for you to fetch changes up to efbc11ccef89030ed54b7368458eeaf9ec687c77: MAINTAINERS: add USB gadget regex to u-boot-dfu tree (2024-05-16 13:28:45 +0200) u-boot-dfu-20240516 - Fix cdns3 low power hang via fast access bit - Multiple dwc3 gadget fixes, mainly for USB support on TI AM6232 - Consistent USB_GADGET_MANUFACTURER for PHYTEC boards - MAINTAINERS file update for u-boot-dfu Aswath Govindraju (1): usb: cdns3: gadget.c: Set fast access bit Benjamin Hahn (1): configs: Make USB_GADGET_MANUFACTURER consistent over all PHYTEC boards Felipe Balbi (4): usb: dwc3: gadget: combine return points into a single one usb: dwc3: gadget: clear SUSPHY bit before ep cmds usb: dwc3: gadget: only resume USB2 PHY in <=HIGHSPEED usb: dwc3: gadget: properly check ep cmd Mattijs Korpershoek (2): MAINTAINERS: add tree link for fastboot MAINTAINERS: add USB gadget regex to u-boot-dfu tree Thinh Nguyen (2): usb: dwc3: gadget: Check ENBLSLPM before sending ep command usb: dwc3: gadget: Disable GUSB2PHYCFG.SUSPHY for End Transfer MAINTAINERS | 4 +++ configs/phycore-imx8mp_defconfig | 2 +- configs/phycore_am64x_a53_defconfig | 2 +- configs/phycore_am64x_r5_defconfig | 2 +- configs/phycore_pcl063_defconfig | 2 +- configs/phycore_pcl063_ull_defconfig | 2 +- drivers/usb/cdns3/gadget.c | 4 +++ drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/gadget.c| 47 +--- 9 files changed, 59 insertions(+), 8 deletions(-)
Re: [v2][PATCH 0/2] MAINTAINERS: updates for fastboot and USB gadget
Hi, On Thu, 16 May 2024 11:15:40 +0200, Mattijs Korpershoek wrote: > I've noticed that I'm missing quite some USB gadget patches such as [1] > since I'm not always cc'ed. > > With Marek, we agreed on the following split: > - I take care of USB gadget side > - Marek handles the rest > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu) [1/2] MAINTAINERS: add tree link for fastboot https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/5738a44f88d65c2847ad204e6ba9f4a5f12f7d4e [2/2] MAINTAINERS: add USB gadget regex to u-boot-dfu tree https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/efbc11ccef89030ed54b7368458eeaf9ec687c77 -- Mattijs
Re: [v2][PATCH 2/2] MAINTAINERS: add USB gadget regex to u-boot-dfu tree
Hi Marek, Thank you for the review. On jeu., mai 16, 2024 at 12:23, Marek Vasut wrote: > On 5/16/24 11:15 AM, Mattijs Korpershoek wrote: >> We try to split work with Marek on USB as following: >> - Mattijs handles USB gadget >> - Marek handles the rest of USB >> >> Add additional gadget patterns to the maintainers file so that I >> get cc'ed more often on USB gadget patches. >> >> Signed-off-by: Mattijs Korpershoek > > Acked-by: Marek Vasut > > You can try and look at the N: instead of F: (see top of MAINTAINERS for > meaning), but that should be separate clean up patch. Noted, will review the differences and consider a later clean up patch!
[v2][PATCH 2/2] MAINTAINERS: add USB gadget regex to u-boot-dfu tree
We try to split work with Marek on USB as following: - Mattijs handles USB gadget - Marek handles the rest of USB Add additional gadget patterns to the maintainers file so that I get cc'ed more often on USB gadget patches. Signed-off-by: Mattijs Korpershoek --- MAINTAINERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 05217db79f7e..6d021763a62a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,8 +1017,11 @@ F: common/update.c F: doc/api/dfu.rst F: doc/usage/dfu.rst F: drivers/dfu/ +F: drivers/usb/*/*gadget* F: drivers/usb/gadget/ F: include/dfu.h +F: include/linux/usb/ch9.h +F: include/linux/usb/gadget.h DRIVER MODEL M: Simon Glass -- 2.44.0
[v2][PATCH 1/2] MAINTAINERS: add tree link for fastboot
Fastboot patches go through the u-boot-dfu tree. Add a link in the maintainers file for it. Signed-off-by: Mattijs Korpershoek --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 6853288975c0..05217db79f7e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1118,6 +1118,7 @@ F:test/py/tests/test_event_dump.py FASTBOOT M: Mattijs Korpershoek S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: cmd/fastboot.c F: doc/android/fastboot*.rst F: include/fastboot.h -- 2.44.0
[v2][PATCH 0/2] MAINTAINERS: updates for fastboot and USB gadget
I've noticed that I'm missing quite some USB gadget patches such as [1] since I'm not always cc'ed. With Marek, we agreed on the following split: - I take care of USB gadget side - Marek handles the rest Update the maintainers file to make sure I get included on gadget related patches. While at it, also update FASTBOOT entry to point towards the u-boot-dfu tree. [1] https://lore.kernel.org/all/20240412202611.3565052-1-alexander.sverd...@siemens.com/ Signed-off-by: Mattijs Korpershoek --- Changes in v2: - Updated commit message (s/usb/USB) for patch 2 - Added ch9 include to USB gadget list - Link to v1: https://lore.kernel.org/r/20240514-gadget-maintainer-v1-0-b170f291d...@baylibre.com --- Mattijs Korpershoek (2): MAINTAINERS: add tree link for fastboot MAINTAINERS: add USB gadget regex to u-boot-dfu tree MAINTAINERS | 4 1 file changed, 4 insertions(+) --- base-commit: c8ffd1356d42223cbb8c86280a083cc3c93e6426 change-id: 20240514-gadget-maintainer-cf40fcf2bab4 Best regards, -- Mattijs Korpershoek
Re: [PATCH 0/6] usb: dwc3: gadget: avoid EP command timeout
Hi, On Fri, 12 Apr 2024 22:26:00 +0200, A. Sverdlin wrote: > From: Alexander Sverdlin > > While there are happy users who successfully have been using DFU on TI > AM62x [1][2], there are also others who were not so lucky up to now [3]. > > I felt into latter category and was wondering why I observe this: > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu) [1/6] usb: dwc3: gadget: combine return points into a single one https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/13395507ca1f48de25e70f4d27f709b1a4fa0a82 [2/6] usb: dwc3: gadget: clear SUSPHY bit before ep cmds https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/967b31c3ccdc09284a4447ebc4577bb7ef06d500 [3/6] usb: dwc3: gadget: only resume USB2 PHY in <=HIGHSPEED https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/d107a5319e20d5e5be3bd8fa298aeca9ef4990a0 [4/6] usb: dwc3: gadget: Check ENBLSLPM before sending ep command https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/db11351a887e20ff86e3a4c1f466d3d8dd42754a [5/6] usb: dwc3: gadget: properly check ep cmd https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/95b4d655a44626f888bf823a0561a175d456d33a [6/6] usb: dwc3: gadget: Disable GUSB2PHYCFG.SUSPHY for End Transfer https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/30f39de786ff3a87006461903e41a48c811ee764 -- Mattijs
[PATCH 2/2] MAINTAINERS: add usb gadget regex to u-boot-dfu tree
We try to split work with Marek on USB as following: - Mattijs handles usb gadget - Marek handles the rest of usb Add additional gadget patterns to the maintainers file so that I get cc'ed more often on USB gadget patches. Signed-off-by: Mattijs Korpershoek --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 05217db79f7e..7fd3dd85e0a9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,8 +1017,10 @@ F: common/update.c F: doc/api/dfu.rst F: doc/usage/dfu.rst F: drivers/dfu/ +F: drivers/usb/*/*gadget* F: drivers/usb/gadget/ F: include/dfu.h +F: include/linux/usb/gadget.h DRIVER MODEL M: Simon Glass -- 2.44.0
[PATCH 1/2] MAINTAINERS: add tree link for fastboot
Fastboot patches go through the u-boot-dfu tree. Add a link in the maintainers file for it. Signed-off-by: Mattijs Korpershoek --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 6853288975c0..05217db79f7e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1118,6 +1118,7 @@ F:test/py/tests/test_event_dump.py FASTBOOT M: Mattijs Korpershoek S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: cmd/fastboot.c F: doc/android/fastboot*.rst F: include/fastboot.h -- 2.44.0
[PATCH 0/2] MAINTAINERS: updates for fastboot and usb gadget
I've noticed that I'm missing quite some usb gadget patches such as [1] since I'm not always cc'ed. With Marek, we agreed on the following split: - I take care of usb gadget side - Marek handles the rest Update the maintainers file to make sure I get included on gadget related patches. While at it, also update FASTBOOT entry to point towards the u-boot-dfu tree. [1] https://lore.kernel.org/all/20240412202611.3565052-1-alexander.sverd...@siemens.com/ Signed-off-by: Mattijs Korpershoek --- Mattijs Korpershoek (2): MAINTAINERS: add tree link for fastboot MAINTAINERS: add usb gadget regex to u-boot-dfu tree MAINTAINERS | 3 +++ 1 file changed, 3 insertions(+) --- base-commit: c8ffd1356d42223cbb8c86280a083cc3c93e6426 change-id: 20240514-gadget-maintainer-cf40fcf2bab4 Best regards, -- Mattijs Korpershoek
Re: [PATCH 6/6] usb: dwc3: gadget: Disable GUSB2PHYCFG.SUSPHY for End Transfer
Hi Alexander,
Thank you for the patch.
On ven., avril 12, 2024 at 22:26, "A. Sverdlin"
wrote:
> From: Thinh Nguyen
>
> Upstream Linux commit 3aa07f72894d.
>
> If there's a disconnection while operating in eSS, there may be a delay
> in VBUS drop response from the connector. In that case, the internal
> link state may drop to operate in usb2 speed while the controller thinks
> the VBUS is still high. The driver must make sure to disable
> GUSB2PHYCFG.SUSPHY when sending endpoint command while in usb2 speed.
> The End Transfer command may be called, and only that command needs to
> go through at this point. Let's keep it simple and unconditionally
> disable GUSB2PHYCFG.SUSPHY whenever we issue the command.
>
> This scenario is not seen in real hardware. In a rare case, our
> prototype type-c controller/interface may have a slow response
> triggerring this issue.
>
> Signed-off-by: Thinh Nguyen
> Link:
> https://lore.kernel.org/r/5651117207803c26e2f22ddf4e5ce9e865dcf7c7.1668045468.git.thinh.ngu...@synopsys.com
> Signed-off-by: Greg Kroah-Hartman
> Signed-off-by: Alexander Sverdlin
I've dropped Greg from the cc list as I understand by [1] that he
prefers to not receives responses on this.
Reviewed-by: Mattijs Korpershoek
[1] https://lore.kernel.org/r/all/2024041354-exciting-suggest-b896@gregkh/
> ---
> drivers/usb/dwc3/gadget.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index c14d7870b9461..debfd4d6781db 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -316,7 +316,8 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
>*
>* DWC_usb3 3.30a and DWC_usb31 1.90a programming guide section 3.2.2
>*/
> - if (dwc->gadget.speed <= USB_SPEED_HIGH) {
> + if (dwc->gadget.speed <= USB_SPEED_HIGH ||
> + DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_ENDTRANSFER) {
> reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
> saved_config |= DWC3_GUSB2PHYCFG_SUSPHY;
> --
> 2.44.0
Re: [PATCH 5/6] usb: dwc3: gadget: properly check ep cmd
Hi Alexander, Thank you for the patch. On ven., avril 12, 2024 at 22:26, "A. Sverdlin" wrote: > From: Felipe Balbi > > Upstream Linux commit 514f227b. > > The cmd argument we pass to > dwc3_send_gadget_ep_cmd() could contain extra > arguments embedded. When checking for StartTransfer > command, we need to make sure to match only lower 4 > bits which contain the actual command and ignore the > rest. > > Reported-by: Janusz Dziedzic > Signed-off-by: Felipe Balbi > [A. Sverdlin: cherry-picked only DWC3_DEPCMD_CMD() define] > Signed-off-by: Alexander Sverdlin Reviewed-by: Mattijs Korpershoek > --- > drivers/usb/dwc3/core.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 1e7eda89a34c9..7709ab793f36d 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -405,6 +405,8 @@ > #define DWC3_DEPCMD_SETTRANSFRESOURCE(0x02 << 0) > #define DWC3_DEPCMD_SETEPCONFIG (0x01 << 0) > > +#define DWC3_DEPCMD_CMD(x) ((x) & 0xf) > + > /* The EP number goes 0..31 so ep0 is always out and ep1 is always in */ > #define DWC3_DALEPENA_EP(n) (1 << n) > > -- > 2.44.0
Re: [PATCH 4/6] usb: dwc3: gadget: Check ENBLSLPM before sending ep command
Hi Alexander,
Thank you for the patch.
On ven., avril 12, 2024 at 22:26, "A. Sverdlin"
wrote:
> From: Thinh Nguyen
>
> Upstream Linux commit 87dd96111b0b.
>
> When operating in USB 2.0 speeds (HS/FS), if GUSB2PHYCFG.ENBLSLPM or
> GUSB2PHYCFG.SUSPHY is set, it must be cleared before issuing an endpoint
> command.
>
> Current implementation only save and restore GUSB2PHYCFG.SUSPHY
> configuration. We must save and clear both GUSB2PHYCFG.ENBLSLPM and
> GUSB2PHYCFG.SUSPHY settings. Restore them after the command is
> completed.
>
> DWC_usb3 3.30a and DWC_usb31 1.90a programming guide section 3.2.2
>
> Signed-off-by: Thinh Nguyen
> Signed-off-by: Felipe Balbi
> Signed-off-by: Alexander Sverdlin
Reviewed-by: Mattijs Korpershoek
> ---
> drivers/usb/dwc3/gadget.c | 29 +++--
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 00845dbadd27a..c14d7870b9461 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -301,26 +301,35 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned
> ep,
> unsigned cmd, struct dwc3_gadget_ep_cmd_params *params)
> {
> u32 timeout = 500;
> + u32 saved_config = 0;
> u32 reg;
>
> - int susphy = false;
> int ret = -EINVAL;
>
> /*
> - * Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
> - * we're issuing an endpoint command, we must check if
> - * GUSB2PHYCFG.SUSPHY bit is set. If it is, then we need to clear it.
> + * When operating in USB 2.0 speeds (HS/FS), if GUSB2PHYCFG.ENBLSLPM or
> + * GUSB2PHYCFG.SUSPHY is set, it must be cleared before issuing an
> + * endpoint command.
>*
> - * We will also set SUSPHY bit to what it was before returning as stated
> - * by the same section on Synopsys databook.
> + * Save and clear both GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY
> + * settings. Restore them after the command is completed.
> + *
> + * DWC_usb3 3.30a and DWC_usb31 1.90a programming guide section 3.2.2
>*/
> if (dwc->gadget.speed <= USB_SPEED_HIGH) {
> reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
> - susphy = true;
> + saved_config |= DWC3_GUSB2PHYCFG_SUSPHY;
> reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> }
> +
> + if (reg & DWC3_GUSB2PHYCFG_ENBLSLPM) {
> + saved_config |= DWC3_GUSB2PHYCFG_ENBLSLPM;
> + reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> + }
> +
> + if (saved_config)
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> }
>
> dwc3_writel(dwc->regs, DWC3_DEPCMDPAR0(ep), params->param0);
> @@ -350,9 +359,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
> udelay(1);
> } while (1);
>
> - if (unlikely(susphy)) {
> + if (saved_config) {
> reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> - reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> + reg |= saved_config;
> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> }
>
> --
> 2.44.0
Re: [PATCH 3/6] usb: dwc3: gadget: only resume USB2 PHY in <=HIGHSPEED
Hi Alexander,
Thank you for the patch.
On ven., avril 12, 2024 at 22:26, "A. Sverdlin"
wrote:
> From: Felipe Balbi
>
> Upstream Linux commit ab2a92e7a608.
>
> As a micro-power optimization, let's only resume the
> USB2 PHY if we're working on <=HIGHSPEED. If we're
> gonna work on SUPERSPEED or SUPERSPEED+, there's no
> point in resuming the USB2 PHY.
>
> Fixes: 2b0f11df84bb ("usb: dwc3: gadget: clear SUSPHY bit before ep cmds")
> Signed-off-by: Felipe Balbi
> Signed-off-by: Alexander Sverdlin
Reviewed-by: Mattijs Korpershoek
> ---
> drivers/usb/dwc3/gadget.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 8f6513752f085..00845dbadd27a 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -314,11 +314,13 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned
> ep,
>* We will also set SUSPHY bit to what it was before returning as stated
>* by the same section on Synopsys databook.
>*/
> - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> - if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
> - susphy = true;
> - reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + if (dwc->gadget.speed <= USB_SPEED_HIGH) {
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
> + susphy = true;
> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + }
> }
>
> dwc3_writel(dwc->regs, DWC3_DEPCMDPAR0(ep), params->param0);
> --
> 2.44.0
Re: [PATCH 2/6] usb: dwc3: gadget: clear SUSPHY bit before ep cmds
Hi Alexander,
Thank you for the patch.
On ven., avril 12, 2024 at 22:26, "A. Sverdlin"
wrote:
> From: Felipe Balbi
>
> Upstream Linux commit 2b0f11df84bb.
>
> Synopsys Databook 2.60a has a note that if we're
> sending an endpoint command we _must_ make sure that
> DWC3_GUSB2PHY(n).SUSPHY bit is cleared.
>
> This patch implements that particular detail.
>
> Signed-off-by: Felipe Balbi
> Signed-off-by: Alexander Sverdlin
Reviewed-by: Mattijs Korpershoek
> ---
> drivers/usb/dwc3/gadget.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 17c19285f1c24..8f6513752f085 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -302,8 +302,25 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned
> ep,
> {
> u32 timeout = 500;
> u32 reg;
> +
> + int susphy = false;
> int ret = -EINVAL;
>
> + /*
> + * Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
> + * we're issuing an endpoint command, we must check if
> + * GUSB2PHYCFG.SUSPHY bit is set. If it is, then we need to clear it.
> + *
> + * We will also set SUSPHY bit to what it was before returning as stated
> + * by the same section on Synopsys databook.
> + */
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
> + susphy = true;
> + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + }
> +
> dwc3_writel(dwc->regs, DWC3_DEPCMDPAR0(ep), params->param0);
> dwc3_writel(dwc->regs, DWC3_DEPCMDPAR1(ep), params->param1);
> dwc3_writel(dwc->regs, DWC3_DEPCMDPAR2(ep), params->param2);
> @@ -331,6 +348,12 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned
> ep,
> udelay(1);
> } while (1);
>
> + if (unlikely(susphy)) {
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> + }
> +
> return ret;
> }
>
> --
> 2.44.0
Re: [PATCH 1/6] usb: dwc3: gadget: combine return points into a single one
Hi Alexander,
Thank you for the patch.
On ven., avril 12, 2024 at 22:26, "A. Sverdlin"
wrote:
> From: Felipe Balbi
>
> Upstream Linux commit c0ca324d09a0.
>
> dwc3_send_gadget_ep_cmd() had three return
> points. That becomes a pain to track when we need to
> debug something or if we need to add more code
> before returning.
>
> Let's combine all three return points into a single
> one just by introducing a local 'ret' variable.
>
> Signed-off-by: Felipe Balbi
> Signed-off-by: Alexander Sverdlin
Reviewed-by: Mattijs Korpershoek
> ---
> drivers/usb/dwc3/gadget.c | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 69d9fe40e2f87..17c19285f1c24 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -302,6 +302,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
> {
> u32 timeout = 500;
> u32 reg;
> + int ret = -EINVAL;
>
> dwc3_writel(dwc->regs, DWC3_DEPCMDPAR0(ep), params->param0);
> dwc3_writel(dwc->regs, DWC3_DEPCMDPAR1(ep), params->param1);
> @@ -313,7 +314,8 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
> if (!(reg & DWC3_DEPCMD_CMDACT)) {
> dev_vdbg(dwc->dev, "Command Complete --> %d\n",
> DWC3_DEPCMD_STATUS(reg));
> - return 0;
> + ret = 0;
> + break;
> }
>
> /*
> @@ -321,11 +323,15 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned
> ep,
>* interrupt context.
>*/
> timeout--;
> - if (!timeout)
> - return -ETIMEDOUT;
> + if (!timeout) {
> + ret = -ETIMEDOUT;
> + break;
> + }
>
> udelay(1);
> } while (1);
> +
> + return ret;
> }
>
> static dma_addr_t dwc3_trb_dma_offset(struct dwc3_ep *dep,
> --
> 2.44.0
Re: [PATCH] configs: Make USB_GADGET_MANUFACTURER consistent over all PHYTEC boards
Hi, On Fri, 03 May 2024 09:00:38 +0200, Benjamin Hahn wrote: > Set CONFIG_USB_GADGET_MANUFACTURER to PHYTEC for all PHYTEC boards. > > Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu) [1/1] configs: Make USB_GADGET_MANUFACTURER consistent over all PHYTEC boards https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/371b379edbf3488d77bc321dc5d2a0e22e067744 -- Mattijs
Re: [PATCH] configs: Make USB_GADGET_MANUFACTURER consistent over all PHYTEC boards
Hi Benjamin, Thank you for the patch. On ven., mai 03, 2024 at 09:00, Benjamin Hahn wrote: > Set CONFIG_USB_GADGET_MANUFACTURER to PHYTEC for all PHYTEC boards. > > Signed-off-by: Benjamin Hahn Reviewed-by: Mattijs Korpershoek This is assigned to me on patchwork. I can pick it up through u-boot-dfu. If someone else wants to pick it up, please let me know. > --- > configs/phycore-imx8mp_defconfig | 2 +- > configs/phycore_am64x_a53_defconfig | 2 +- > configs/phycore_am64x_r5_defconfig | 2 +- > configs/phycore_pcl063_defconfig | 2 +- > configs/phycore_pcl063_ull_defconfig | 2 +- > 5 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/configs/phycore-imx8mp_defconfig > b/configs/phycore-imx8mp_defconfig > index e9a287cb441f..9f42edd72324 100644 > --- a/configs/phycore-imx8mp_defconfig > +++ b/configs/phycore-imx8mp_defconfig > @@ -147,7 +147,7 @@ CONFIG_USB_DWC3=y > CONFIG_USB_DWC3_GENERIC=y > CONFIG_USB_STORAGE=y > CONFIG_USB_GADGET=y > -CONFIG_USB_GADGET_MANUFACTURER="FSL" > +CONFIG_USB_GADGET_MANUFACTURER="PHYTEC" > CONFIG_USB_GADGET_VENDOR_NUM=0x0525 > CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5 > CONFIG_IMX_WATCHDOG=y > diff --git a/configs/phycore_am64x_a53_defconfig > b/configs/phycore_am64x_a53_defconfig > index 1a9359773b45..9b52f8ad0644 100644 > --- a/configs/phycore_am64x_a53_defconfig > +++ b/configs/phycore_am64x_a53_defconfig > @@ -159,7 +159,7 @@ CONFIG_USB_CDNS3=y > CONFIG_USB_CDNS3_GADGET=y > CONFIG_USB_CDNS3_HOST=y > CONFIG_USB_GADGET=y > -CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments" > +CONFIG_USB_GADGET_MANUFACTURER="PHYTEC" > CONFIG_USB_GADGET_VENDOR_NUM=0x0451 > CONFIG_USB_GADGET_PRODUCT_NUM=0x6165 > CONFIG_USB_GADGET_DOWNLOAD=y > diff --git a/configs/phycore_am64x_r5_defconfig > b/configs/phycore_am64x_r5_defconfig > index 61d784fa17f6..15a7e7089e73 100644 > --- a/configs/phycore_am64x_r5_defconfig > +++ b/configs/phycore_am64x_r5_defconfig > @@ -171,7 +171,7 @@ CONFIG_USB_STORAGE=y > CONFIG_SPL_USB_STORAGE=y > CONFIG_USB_GADGET=y > CONFIG_SPL_USB_GADGET=y > -CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments" > +CONFIG_USB_GADGET_MANUFACTURER="PHYTEC" > CONFIG_USB_GADGET_VENDOR_NUM=0x0451 > CONFIG_USB_GADGET_PRODUCT_NUM=0x6165 > CONFIG_USB_GADGET_DOWNLOAD=y > diff --git a/configs/phycore_pcl063_defconfig > b/configs/phycore_pcl063_defconfig > index 017054a8e12b..2f6b158a6772 100644 > --- a/configs/phycore_pcl063_defconfig > +++ b/configs/phycore_pcl063_defconfig > @@ -62,7 +62,7 @@ CONFIG_IMX_THERMAL=y > CONFIG_USB=y > CONFIG_SPL_USB_HOST=y > CONFIG_USB_GADGET=y > -CONFIG_USB_GADGET_MANUFACTURER="Phytec" > +CONFIG_USB_GADGET_MANUFACTURER="PHYTEC" > CONFIG_USB_GADGET_VENDOR_NUM=0x1b67 > CONFIG_USB_GADGET_PRODUCT_NUM=0x4fff > CONFIG_CI_UDC=y > diff --git a/configs/phycore_pcl063_ull_defconfig > b/configs/phycore_pcl063_ull_defconfig > index b3da43a5bf1e..b42a410da69c 100644 > --- a/configs/phycore_pcl063_ull_defconfig > +++ b/configs/phycore_pcl063_ull_defconfig > @@ -53,7 +53,7 @@ CONFIG_IMX_THERMAL=y > CONFIG_USB=y > CONFIG_SPL_USB_HOST=y > CONFIG_USB_GADGET=y > -CONFIG_USB_GADGET_MANUFACTURER="Phytec" > +CONFIG_USB_GADGET_MANUFACTURER="PHYTEC" > CONFIG_USB_GADGET_VENDOR_NUM=0x1b67 > CONFIG_USB_GADGET_PRODUCT_NUM=0x4fff > CONFIG_CI_UDC=y > > --- > base-commit: 6fdb021f148f598a67eb3cac5e3eb4a569cdaacd > change-id: 20240503-wip-bhahn-bspimx8m-3196-c1ebd0bab6ac > > Best regards, > -- > Benjamin Hahn
Re: [PATCH v5 5/6] beagleplay: Add DFU support
Hi Martyn,
Thank you for the patch, and for taking over this series.
On lun., mai 06, 2024 at 15:38, Martyn Welch wrote:
> From: Sjoerd Simons
>
> DFU mode on a beagleplay can be used via the Type-C connector by holding
> the USR switch while powering on.
>
> Configuration is already provided as fragments for both the A53 and R5
> u-boot parts. Include the am62x_a53_usbdfu.config config. The
> am62x_r5_usbdfu.config fragment needs to be added should DFU boot be
> required as this will disable booting from persistent storage due to
> binary size constraints.
>
> Signed-off-by: Sjoerd Simons
> Signed-off-by: Martyn Welch
Reviewed-by: Mattijs Korpershoek
I've tested usb gadget on Beagle Play board using fastboot.
Also tested DFU mode via snagboot.
Tested-by: Mattijs Korpershoek
> ---
> Changes in v5:
> - Use existing config fragment for a53 DFU configuration
> - Force usb0 into peripheral mode
>
> Changes in v4:
> - New patch
>
> arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi | 9 +
> board/beagle/beagleplay/beagleplay.env | 1 +
> configs/am62x_beagleplay_a53_defconfig | 2 ++
> 3 files changed, 12 insertions(+)
>
> diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> index fb2032068d..967a2bdcd1 100644
> --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> @@ -54,6 +54,15 @@
> >;
> };
>
> + {
> + bootph-all;
> +};
> +
> + {
> + dr_mode = "peripheral";
> + bootph-all;
> +};
> +
> #ifdef CONFIG_TARGET_AM625_A53_BEAGLEPLAY
>
> #define SPL_NODTB "spl/u-boot-spl-nodtb.bin"
> diff --git a/board/beagle/beagleplay/beagleplay.env
> b/board/beagle/beagleplay/beagleplay.env
> index bbf6b925d0..8dbfc2f7d2 100644
> --- a/board/beagle/beagleplay/beagleplay.env
> +++ b/board/beagle/beagleplay/beagleplay.env
> @@ -1,5 +1,6 @@
> #include
> #include
> +#include
>
> name_kern=Image
> console=ttyS2,115200n8
> diff --git a/configs/am62x_beagleplay_a53_defconfig
> b/configs/am62x_beagleplay_a53_defconfig
> index 4f1be1df59..ec62670d55 100644
> --- a/configs/am62x_beagleplay_a53_defconfig
> +++ b/configs/am62x_beagleplay_a53_defconfig
> @@ -121,3 +121,5 @@ CONFIG_EXT4_WRITE=y
> CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
> CONFIG_LZO=y
> CONFIG_EFI_SET_TIME=y
> +
> +#include
> --
> 2.43.0
Re: [PATCH v5 4/6] configs: am62x_evm_*: Enable USB and DFU support
Hi Martyn, Thank you for the patch. On lun., mai 06, 2024 at 15:38, Martyn Welch wrote: > From: Sjoerd Simons > > Provide config fragments to enable USB host as well as USB gadget and DFU > support for a53 and r5. This relevant fragment is included into the > am62x EVM a53 defconfig. For the r5, due to the smaller available size, > the config fragment also disables support for persistent storage to free > up space for USB support. This fragment needs to be included is DFU > booting is desired. > > The CONFIG_DFU_SF option is placed in the defconfig rather than the > fragment as this is known not to be supported on all boards that can > support DFU. > > Signed-off-by: Sjoerd Simons > Signed-off-by: Martyn Welch Reviewed-by: Mattijs Korpershoek > --- > Changes in v5: > - Switch to config fragment for a53 most DFU configuration > > Changes in v4: > - Move R5 dfu config to a config fragment rather then a full defconfig > - Don't enable XHCI for the R5 SPL, unneeded > > Changes in v3: > - Run savedefconfig to adjust to more recent u-boot > > Changes in v2: > - Create a seperate defconfig for R5 > > > > configs/am62x_a53_usbdfu.config | 30 ++ > configs/am62x_evm_a53_defconfig | 2 ++ > configs/am62x_r5_usbdfu.config | 28 > 3 files changed, 60 insertions(+) > create mode 100644 configs/am62x_a53_usbdfu.config > create mode 100644 configs/am62x_r5_usbdfu.config > > diff --git a/configs/am62x_a53_usbdfu.config b/configs/am62x_a53_usbdfu.config > new file mode 100644 > index 00..3a19cf2328 > --- /dev/null > +++ b/configs/am62x_a53_usbdfu.config > @@ -0,0 +1,29 @@ > +CONFIG_SYS_MALLOC_LEN=0x200 > +CONFIG_SPL_ENV_SUPPORT=y > +CONFIG_SPL_RAM_SUPPORT=y > +CONFIG_SPL_RAM_DEVICE=y > +CONFIG_SPL_USB_GADGET=y > +CONFIG_SPL_DFU=y > +CONFIG_CMD_DFU=y > +CONFIG_CMD_USB=y > +CONFIG_SYSCON=y > +CONFIG_SPL_SYSCON=y > +CONFIG_DFU_MMC=y > +CONFIG_DFU_RAM=y > +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000 > +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x80 > +CONFIG_USB=y > +CONFIG_DM_USB_GADGET=y > +CONFIG_SPL_DM_USB_GADGET=y > +CONFIG_USB_XHCI_HCD=y > +CONFIG_USB_XHCI_DWC3=y > +CONFIG_USB_DWC3=y > +CONFIG_USB_DWC3_GENERIC=y > +CONFIG_SPL_USB_DWC3_GENERIC=y > +CONFIG_SPL_USB_DWC3_AM62=y > +CONFIG_USB_DWC3_AM62=y > +CONFIG_USB_GADGET=y > +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments" > +CONFIG_USB_GADGET_VENDOR_NUM=0x0451 > +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165 > +CONFIG_USB_GADGET_DOWNLOAD=y > diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig > index 6c708dcb05..16294a6a79 100644 > --- a/configs/am62x_evm_a53_defconfig > +++ b/configs/am62x_evm_a53_defconfig > @@ -68,6 +68,7 @@ CONFIG_SPL_OF_TRANSLATE=y > CONFIG_CLK=y > CONFIG_SPL_CLK=y > CONFIG_CLK_TI_SCI=y > +CONFIG_DFU_SF=y > CONFIG_DMA_CHANNELS=y > CONFIG_TI_K3_NAVSS_UDMA=y > CONFIG_TI_SCI_PROTOCOL=y > @@ -111,3 +112,5 @@ CONFIG_SPL_SYSRESET=y > CONFIG_SYSRESET_TI_SCI=y > CONFIG_FS_FAT_MAX_CLUSTSIZE=16384 > CONFIG_EFI_SET_TIME=y > + > +#include > diff --git a/configs/am62x_r5_usbdfu.config b/configs/am62x_r5_usbdfu.config > new file mode 100644 > index 00..772bb2ab93 > --- /dev/null > +++ b/configs/am62x_r5_usbdfu.config > @@ -0,0 +1,28 @@ > +CONFIG_SPL_ENV_SUPPORT=y > +CONFIG_SYSCON=y > +CONFIG_SPL_SYSCON=y > +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000 > +CONFIG_MISC=y > +CONFIG_USB=y > +CONFIG_DM_USB_GADGET=y > +CONFIG_SPL_DM_USB_GADGET=y > +CONFIG_USB_DWC3=y > +CONFIG_USB_DWC3_GENERIC=y > +CONFIG_SPL_USB_DWC3_GENERIC=y > +CONFIG_SPL_USB_DWC3_AM62=y > +CONFIG_USB_GADGET=y > +CONFIG_SPL_USB_GADGET=y > +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments" > +CONFIG_USB_GADGET_VENDOR_NUM=0x0451 > +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165 > +CONFIG_USB_GADGET_DOWNLOAD=y > +CONFIG_SPL_DFU=y > +# CONFIG_SPL_MMC is not set > +# CONFIG_SPL_FS_FAT is not set > +# CONFIG_SPL_LIBDISK_SUPPORT is not set > +# CONFIG_SPL_SPI is not set > +# CONFIG_SPL_SYS_MALLOC is not set > +# CONFIG_CMD_GPT is not set > +# CONFIG_CMD_MMC is not set > +# CONFIG_CMD_FAT is not set > +# CONFIG_MMC_SDHCI is not set > -- > 2.43.0
Re: [PATCH v5 3/6] arm: dts: k3-am625-sk: Enable usb port in u-boot
Hi Martyn,
Thank you for the patch.
On lun., mai 06, 2024 at 15:38, Martyn Welch wrote:
> From: Sjoerd Simons
>
> Enable usb0 in all boot phases for use with DFU
>
> Signed-off-by: Sjoerd Simons
> Reviewed-by: Alexander Sverdlin
> Signed-off-by: Martyn Welch
Reviewed-by: Mattijs Korpershoek
> ---
> Changes in v5:
> - Forcing usb0 into peripheral mode reinstated
>
> Changes in v4:
> - Don't force usb0 into peripheral mode
>
> Changes in v3:
> - Enable usb nodes in all boot phases
>
> Changes in v2:
> - Only enable usb port 0 DFU in SPL
>
> arch/arm/dts/k3-am625-sk-u-boot.dtsi | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> index fa778b0ff4..1fc0d407cb 100644
> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> @@ -46,3 +46,12 @@
> _port2 {
> status = "disabled";
> };
> +
> + {
> + bootph-all;
> +};
> +
> + {
> + dr_mode = "peripheral";
> + bootph-all;
> +};
> --
> 2.43.0
Re: [PATCH 73/81] usb: Remove and add needed includes
Hi Tom,
On jeu., mai 02, 2024 at 08:51, Tom Rini wrote:
> On Thu, May 02, 2024 at 09:40:52AM +0200, Mattijs Korpershoek wrote:
>> Hi Tom,
>>
>> Thank you for the patch
>>
>> On mer., mai 01, 2024 at 19:31, Tom Rini wrote:
>>
>> > Remove from this driver directory and when needed
>> > add missing include files directly.
>> >
>> > Signed-off-by: Tom Rini
>>
>> [...]
>>
>> > diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
>> > index fedcf7869295..38c5928faed5 100644
>> > --- a/drivers/usb/host/xhci-rcar.c
>> > +++ b/drivers/usb/host/xhci-rcar.c
>> > @@ -5,7 +5,6 @@
>> > * Renesas RCar USB HOST xHCI Controller
>> > */
>> >
>> > -#include
>> > #include
>> > #include
>> > #include
>> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> > index 910c5f3352b8..1360a5940fa0 100644
>> > --- a/drivers/usb/host/xhci-ring.c
>> > +++ b/drivers/usb/host/xhci-ring.c
>> > @@ -13,7 +13,6 @@
>> > *Vikas Sajjan
>> > */
>> >
>> > -#include
>>
>> This generates the following build warning with
>> khadas-vim3_android_defconfig:
>>
>> drivers/usb/host/xhci-ring.c: In function 'xhci_wait_for_event':
>> drivers/usb/host/xhci-ring.c:464:28: warning: implicit declaration of
>> function 'get_timer'; did you mean 'get_mem'?
>> [-Wimplicit-function-declaration]
>> 464 | unsigned long ts = get_timer(0);
>> |^
>> |get_mem
>>
>> Adding: "#include " fixes the warning.
>>
>> With the above fix included:
>>
>> Reviewed-by: Mattijs Korpershoek
>
> Did you have the full series applied? I don't see the warning here (nor
> in CI) and I suspect that:
> https://patchwork.ozlabs.org/project/uboot/patch/20240502013138.2383421-9-tr...@konsulko.com/
> is what resolves this warning.
No, I did not apply the full series. I had some conflicts on both:
- master: ff0de1f0557e ("Merge patch series "Update PHYTEC SOM Detection"")
- next: bc39e0677816 ("Subtree merge tag 'v6.8-dts' of devicetree-rebasing repo
[1] into dts/upstream")
The above patch resolves the warning, indeed!
Thanks
>
> --
> Tom
Re: [PATCH 73/81] usb: Remove and add needed includes
Hi Tom, Thank you for the patch On mer., mai 01, 2024 at 19:31, Tom Rini wrote: > Remove from this driver directory and when needed > add missing include files directly. > > Signed-off-by: Tom Rini [...] > diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c > index fedcf7869295..38c5928faed5 100644 > --- a/drivers/usb/host/xhci-rcar.c > +++ b/drivers/usb/host/xhci-rcar.c > @@ -5,7 +5,6 @@ > * Renesas RCar USB HOST xHCI Controller > */ > > -#include > #include > #include > #include > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index 910c5f3352b8..1360a5940fa0 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -13,7 +13,6 @@ > * Vikas Sajjan > */ > > -#include This generates the following build warning with khadas-vim3_android_defconfig: drivers/usb/host/xhci-ring.c: In function 'xhci_wait_for_event': drivers/usb/host/xhci-ring.c:464:28: warning: implicit declaration of function 'get_timer'; did you mean 'get_mem'? [-Wimplicit-function-declaration] 464 | unsigned long ts = get_timer(0); |^ |get_mem Adding: "#include " fixes the warning. With the above fix included: Reviewed-by: Mattijs Korpershoek > #include > #include > #include > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 741e186ee05b..d30725d3fcaa 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -19,7 +19,6 @@ > * The quirk devices support hasn't been given yet. > */ > > -#include > #include > #include > #include [...]
Re: [PATCH 28/81] fastboot: Remove and add needed includes
Hi Tom, Thank you for the patch. On mer., mai 01, 2024 at 19:30, Tom Rini wrote: > Remove from this driver directory and when needed > add missing include files directly. > > Signed-off-by: Tom Rini Reviewed-by: Mattijs Korpershoek > --- > Cc: Mattijs Korpershoek > Cc: Tom Rini > --- > drivers/fastboot/fb_command.c | 2 +- > drivers/fastboot/fb_common.c | 2 +- > drivers/fastboot/fb_getvar.c | 2 +- > drivers/fastboot/fb_mmc.c | 1 - > drivers/fastboot/fb_nand.c| 1 - > 5 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c > index 01443c5d39e2..e4484d65aca5 100644 > --- a/drivers/fastboot/fb_command.c > +++ b/drivers/fastboot/fb_command.c > @@ -3,7 +3,6 @@ > * Copyright (C) 2016 The Android Open Source Project > */ > > -#include > #include > #include > #include > @@ -13,6 +12,7 @@ > #include > #include > #include > +#include > #include > > /** > diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c > index 3576b0677299..12ffb463deb9 100644 > --- a/drivers/fastboot/fb_common.c > +++ b/drivers/fastboot/fb_common.c > @@ -11,11 +11,11 @@ > */ > > #include > -#include > #include > #include > #include > #include > +#include > > /** > * fastboot_buf_addr - base address of the fastboot download buffer > diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c > index f65519c57b47..93cbd598e024 100644 > --- a/drivers/fastboot/fb_getvar.c > +++ b/drivers/fastboot/fb_getvar.c > @@ -3,7 +3,6 @@ > * Copyright (C) 2016 The Android Open Source Project > */ > > -#include > #include > #include > #include > @@ -12,6 +11,7 @@ > #include > #include > #include > +#include > #include > > static void getvar_version(char *var_parameter, char *response); > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c > index 060918e49109..f11eb66761b1 100644 > --- a/drivers/fastboot/fb_mmc.c > +++ b/drivers/fastboot/fb_mmc.c > @@ -4,7 +4,6 @@ > */ > > #include > -#include > #include > #include > #include > diff --git a/drivers/fastboot/fb_nand.c b/drivers/fastboot/fb_nand.c > index bbe26ddcc9be..afc64fd52807 100644 > --- a/drivers/fastboot/fb_nand.c > +++ b/drivers/fastboot/fb_nand.c > @@ -5,7 +5,6 @@ > */ > > #include > -#include > #include > > #include > -- > 2.34.1
Re: [PATCH 25/81] dfu: Remove and add needed includes
Hi Tom, Thank you for the patch. On mer., mai 01, 2024 at 19:30, Tom Rini wrote: > Remove from this driver directory and when needed > add missing include files directly. > > Signed-off-by: Tom Rini Reviewed-by: Mattijs Korpershoek Since this is quite a big series, I'm assuming you will pick it up yourself. If it's expected of me to pick this up through u-boot-dfu, let me know. > --- > Cc: Lukasz Majewski > Cc: Mattijs Korpershoek > Cc: Tom Rini > --- > drivers/dfu/dfu.c | 1 - > drivers/dfu/dfu_alt.c | 1 - > drivers/dfu/dfu_mmc.c | 1 - > drivers/dfu/dfu_mtd.c | 1 - > drivers/dfu/dfu_nand.c | 1 - > drivers/dfu/dfu_ram.c | 1 - > drivers/dfu/dfu_sf.c | 1 - > drivers/dfu/dfu_virt.c | 1 - > 8 files changed, 8 deletions(-) > > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c > index 2adf26e2fe24..540d48fab77d 100644 > --- a/drivers/dfu/dfu.c > +++ b/drivers/dfu/dfu.c > @@ -6,7 +6,6 @@ > * author: Lukasz Majewski > */ > > -#include > #include > #include > #include > diff --git a/drivers/dfu/dfu_alt.c b/drivers/dfu/dfu_alt.c > index ece3d2236f3d..e9132936a90b 100644 > --- a/drivers/dfu/dfu_alt.c > +++ b/drivers/dfu/dfu_alt.c > @@ -4,7 +4,6 @@ > * Lukasz Majewski > */ > > -#include > #include > #include > #include > diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > index 12c54e90ef71..cfa6334e4397 100644 > --- a/drivers/dfu/dfu_mmc.c > +++ b/drivers/dfu/dfu_mmc.c > @@ -6,7 +6,6 @@ > * author: Lukasz Majewski > */ > > -#include > #include > #include > #include > diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c > index 485586989c8a..c36ac09189f3 100644 > --- a/drivers/dfu/dfu_mtd.c > +++ b/drivers/dfu/dfu_mtd.c > @@ -7,7 +7,6 @@ > * Based on dfu_nand.c > */ > > -#include > #include > #include > #include > diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c > index 08e8cf5cdb37..940cfefc986c 100644 > --- a/drivers/dfu/dfu_nand.c > +++ b/drivers/dfu/dfu_nand.c > @@ -9,7 +9,6 @@ > * author: Lukasz Majewski > */ > > -#include > #include > #include > #include > diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c > index c4f4bd2e482f..043acbf022fc 100644 > --- a/drivers/dfu/dfu_ram.c > +++ b/drivers/dfu/dfu_ram.c > @@ -8,7 +8,6 @@ > * author: Lukasz Majewski > */ > > -#include > #include > #include > #include > diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c > index 2dae15937064..7c1c0f9e2dc7 100644 > --- a/drivers/dfu/dfu_sf.c > +++ b/drivers/dfu/dfu_sf.c > @@ -3,7 +3,6 @@ > * Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved. > */ > > -#include > #include > #include > #include > diff --git a/drivers/dfu/dfu_virt.c b/drivers/dfu/dfu_virt.c > index 29f7a08f6728..2c31445af12a 100644 > --- a/drivers/dfu/dfu_virt.c > +++ b/drivers/dfu/dfu_virt.c > @@ -2,7 +2,6 @@ > /* > * Copyright (C) 2019, STMicroelectronics - All Rights Reserved > */ > -#include > #include > #include > #include > -- > 2.34.1
Re: [PATCH 14/81] block: Remove and add needed includes
Hi Tom,
Thank you for the patch.
On mer., mai 01, 2024 at 19:30, Tom Rini wrote:
> Remove from this driver directory and when needed
> add missing include files directly.
>
> Signed-off-by: Tom Rini
Reviewed-by: Mattijs Korpershoek
> ---
> Cc: Tom Rini
> Cc: Tobias Waldekranz
> Cc: Simon Glass
> Cc: Heinrich Schuchardt
> Cc: Marek Vasut
> Cc: Bin Meng
> Cc: Johan Jonker
> Cc: Kever Yang
> Cc: Dan Carpenter
> Cc: Mattijs Korpershoek
> ---
> drivers/block/blk-uclass.c | 1 -
> drivers/block/blk_legacy.c | 1 -
> drivers/block/blkcache.c | 1 -
> drivers/block/blkmap.c | 1 -
> drivers/block/efi-media-uclass.c | 1 -
> drivers/block/efi_blk.c | 1 -
> drivers/block/host-uclass.c | 1 -
> drivers/block/host_dev.c | 1 -
> drivers/block/ide.c | 1 -
> drivers/block/sandbox.c | 1 -
> drivers/block/sb_efi_media.c | 1 -
> 11 files changed, 11 deletions(-)
>
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 77066da352a3..512c952f4d7a 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -6,7 +6,6 @@
>
> #define LOG_CATEGORY UCLASS_BLK
>
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/blk_legacy.c b/drivers/block/blk_legacy.c
> index 5bf1d0471524..f36932183d1f 100644
> --- a/drivers/block/blk_legacy.c
> +++ b/drivers/block/blk_legacy.c
> @@ -4,7 +4,6 @@
> * Written by Simon Glass
> */
>
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/blkcache.c b/drivers/block/blkcache.c
> index 26bcbea43533..0e69160249c7 100644
> --- a/drivers/block/blkcache.c
> +++ b/drivers/block/blkcache.c
> @@ -4,7 +4,6 @@
> * Author: Eric Nelson
> *
> */
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> index 21201409ed4b..34eed1380dca 100644
> --- a/drivers/block/blkmap.c
> +++ b/drivers/block/blkmap.c
> @@ -4,7 +4,6 @@
> * Author: Tobias Waldekranz
> */
>
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/efi-media-uclass.c
> b/drivers/block/efi-media-uclass.c
> index e012f6f2f4c4..dc5e4f59b7f3 100644
> --- a/drivers/block/efi-media-uclass.c
> +++ b/drivers/block/efi-media-uclass.c
> @@ -5,7 +5,6 @@
> * Copyright 2021 Google LLC
> */
>
> -#include
> #include
>
> UCLASS_DRIVER(efi_media) = {
> diff --git a/drivers/block/efi_blk.c b/drivers/block/efi_blk.c
> index 917a19f60254..9766cd6f8327 100644
> --- a/drivers/block/efi_blk.c
> +++ b/drivers/block/efi_blk.c
> @@ -8,7 +8,6 @@
> * Copyright 2021 Google LLC
> */
>
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/host-uclass.c b/drivers/block/host-uclass.c
> index b3647e3ce335..cf42bd1e07ac 100644
> --- a/drivers/block/host-uclass.c
> +++ b/drivers/block/host-uclass.c
> @@ -9,7 +9,6 @@
>
> #define LOG_CATEGORY UCLASS_HOST
>
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/host_dev.c b/drivers/block/host_dev.c
> index 52313435a0cb..b3ff3cd1fab9 100644
> --- a/drivers/block/host_dev.c
> +++ b/drivers/block/host_dev.c
> @@ -9,7 +9,6 @@
>
> #define LOG_CATEGORY UCLASS_HOST
>
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/ide.c b/drivers/block/ide.c
> index c698f9cbd558..b16623d7a3ab 100644
> --- a/drivers/block/ide.c
> +++ b/drivers/block/ide.c
> @@ -6,7 +6,6 @@
>
> #define LOG_CATEGORY UCLASS_IDE
>
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
> index be4e02cb601a..ec34f1ad8c2e 100644
> --- a/drivers/block/sandbox.c
> +++ b/drivers/block/sandbox.c
> @@ -3,7 +3,6 @@
> * Copyright (C) 2013 Henrik Nordstrom
> */
>
> -#include
> #include
> #include
> #include
> diff --git a/drivers/block/sb_efi_media.c b/drivers/block/sb_efi_media.c
> index 52af155a6001..3255db064961 100644
> --- a/drivers/block/sb_efi_media.c
> +++ b/drivers/block/sb_efi_media.c
> @@ -5,7 +5,6 @@
> * Copyright 2021 Google LLC
> */
>
> -#include
> #include
>
> static const struct udevice_id sandbox_efi_media_ids[] = {
> --
> 2.34.1
Re: [PATCH v4 1/7] usb: dwc3: Add dwc3 glue driver for am62
Hi Martyn,
On mer., mai 01, 2024 at 14:56, Martyn Welch wrote:
> On Tue, 2024-01-16 at 11:22 +0100, Mattijs Korpershoek wrote:
>> > + /* Program PHY PLL refclk by reading syscon property */
>> > + ret = regmap_update_bits(syscon, args.args[0],
>> > PHY_PLL_REFCLK_MASK, rate_code);
>> > + if (ret) {
>>
>> The doc of ofnode_parse_phandle_with_args() states that:
>>
>> * Caller is responsible to call of_node_put() on the returned
>> out_args->np
>> * pointer.
>>
>> Should we call of_node_put(args->np); before returning here?
>>
>
> It doesn't seem that this is done in U-Boot as the definition of
> of_node_put() here is:
>
>/* Dummy functions to mirror Linux. These are not used in U-Boot */
>#define of_node_get(x) (x)
>static inline void of_node_put(const struct device_node *np) { }
Indeed, thank you for looking into this.
In that case,
Reviewed-by: Mattijs Korpershoek
>
> Martyn
>
>> Should the cleanup be done in case of success as well?
>>
>> With that fixed:
>>
>> Reviewed-by: Mattijs Korpershoek
>>
Re: [PATCH v3] usb: cdns3: gadget.c: Set fast access bit
Hi, On Wed, 24 Apr 2024 13:09:11 +0530, Ravi Gunasekaran wrote: > When the device port is in a low power state [U3/L2/Not Connected], > accesses to usb device registers may take a long time. This could lead to > potential core hang when the controller registers are accessed after the > port is disabled by setting DEVDS field. Setting the fast register access > bit ensures that the PHY clock is keeping up in active state. > > Therefore, set fast access bit to ensure the accesses to device registers > are quick even in low power states. > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu) [1/1] usb: cdns3: gadget.c: Set fast access bit https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/3f831439c2dddcb84634195263e619d742e196a3 -- Mattijs
Re: [PATCH v3] usb: cdns3: gadget.c: Set fast access bit
Hi Ravi,
Thank you for the patch.
On mer., avril 24, 2024 at 14:15, Marek Vasut wrote:
> On 4/24/24 9:39 AM, Ravi Gunasekaran wrote:
>> From: Aswath Govindraju
>>
>> When the device port is in a low power state [U3/L2/Not Connected],
>> accesses to usb device registers may take a long time. This could lead to
>> potential core hang when the controller registers are accessed after the
>> port is disabled by setting DEVDS field. Setting the fast register access
>> bit ensures that the PHY clock is keeping up in active state.
>>
>> Therefore, set fast access bit to ensure the accesses to device registers
>> are quick even in low power states.
>>
>> commit b5148d946f45 ("usb: cdns3: gadget: set fast access bit") in the
>> upstream kernel is taken as reference.
>>
>> Signed-off-by: Aswath Govindraju
>> Signed-off-by: Ravi Gunasekaran
>> Reviewed-by: Roger Quadros
>> Reviewed-by: Marek Vasut
>> ---
>> Changes since v2:
>>
>> * Updated the commit description with reference to upstream kernel commit
>> * Picked up Marek's Reviewed-by tag from v2
>>
>> Changes since v1:
>> -
>> * Rebased to tip of -next
>> * Added Reviewed-by tag
>>
>> v2: https://lore.kernel.org/all/20240423082420.32336-1-r-gunaseka...@ti.com/
>> v1:
>> https://lore.kernel.org/u-boot/20230505121353.23486-1-r-gunaseka...@ti.com/
>>
>> drivers/usb/cdns3/gadget.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> index 7aa0c6b2be..d11175dc5b 100644
>> --- a/drivers/usb/cdns3/gadget.c
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -2325,6 +2325,9 @@ static void cdns3_gadget_config(struct cdns3_device
>> *priv_dev)
>> writel(USB_IEN_INIT, >usb_ien);
>> writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, >usb_conf);
>>
>> +/* Set the Fast access bit */
>> +writel(PUSB_PWR_FST_REG_ACCESS, _dev->regs->usb_pwr);
>> +
>> cdns3_configure_dmult(priv_dev, NULL);
>>
>> cdns3_gadget_pullup(_dev->gadget, 1);
>> @@ -2383,6 +2386,7 @@ static int cdns3_gadget_udc_stop(struct usb_gadget
>> *gadget)
>>
>> /* disable interrupt for device */
>> writel(0, _dev->regs->usb_ien);
>> +writel(0, _dev->regs->usb_pwr);
>> writel(USB_CONF_DEVDS, _dev->regs->usb_conf);
>>
>> return ret;
>>
>> base-commit: bc39e06778168a34bb4e0a34fbee4edbde4414d8
>
> Since this is gadget stuff, +CC Mattijs , that should go through their tree.
>
> Thanks!
Thank you Marek for CC'ing me, I missed this patch.
Reviewed-by: Mattijs Korpershoek
Re: [PATCH] usb: dwc2: Add in version 4xx compatibility
Hi Greg, On ven., avril 19, 2024 at 15:21, Greg Malysa wrote: > Hi Mattijs, > >> Please avoid top-posting when replying, it makes following the >> discussion more difficult: >> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions > > Will do. Sorry about that; I'm still learning about this approach to email. No worries. There are quite some things to learn and we probably all did this wrong when starting. Thank you for taking the time to learn and to contribute! > >> Looking at >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c >> >> we can see that the following is added: >> #define DWC2_CORE_REV_MASK 0x >> >> This makes me believe that the versioning follows a well known pattern. > > I can submit a v2 next week to bring it in line with the kernel's approach. To me, it's fine as is. > >> Note that this change is also part of: >> https://lore.kernel.org/all/20240328131811.94559-1-seashell11234...@gmail.com/ > > Or if you prefer I can also drop our patch and we can pursue this > linked patch with both 4xx compatibility and the 420a reset handling. I think the patch you submitted is fine by itself, but I'd let Marek decide since he is the maintainer for this part. If you have access to the hardware that has a 4.20a dwc2 controller, maybe you can help testing the patch above patch as well? > > Thanks, > Greg
[GIT PULL] Please pull u-boot-dfu-20240419
Hi Tom, Here is a new development for master: - new "fastboot oem board" command This CI job is at https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/20427 Thanks, Mattijs The following changes since commit 2c3fa4b8add3cb6a440184ab67debc6867d383c0: sandbox: don't call os_close with invalid file descriptor (2024-04-17 17:06:16 -0600) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-dfu.git tags/u-boot-dfu-20240419 for you to fetch changes up to b2acf59baf917c3b4002c1b2094ddb46c03ab02e: fastboot: introduce 'oem board' subcommand (2024-04-18 14:54:38 +0200) u-boot-dfu-20240419 - new "fastboot oem board" command Alexey Romanov (1): fastboot: introduce 'oem board' subcommand doc/android/fastboot.rst | 18 ++ drivers/fastboot/Kconfig | 7 +++ drivers/fastboot/fb_command.c | 30 ++ include/fastboot.h| 1 + 4 files changed, 56 insertions(+)
Re: [PATCH v5 0/1] Introduce fastboot oem board command
Hi, On Thu, 18 Apr 2024 13:01:28 +0300, Alexey Romanov wrote: > Changes V1 -> V2 [1]: > - Added an example of using the command as requested > by Sean Anderson [2]. > > Changes V2 -> V3 [3]: > - Rebase over uboot/master. > - Add documentation. > - Remove example added in V2 [1]. > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu) [1/1] fastboot: introduce 'oem board' subcommand https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/b2acf59baf917c3b4002c1b2094ddb46c03ab02e -- Mattijs
Re: [PATCH v4 1/1] fastboot: introduce 'oem board' subcommand
Hi Alexey,
On mer., avril 10, 2024 at 13:58, Alexey Romanov
wrote:
> Currently, fastboot protocol in U-Boot has no opportunity
> to execute vendor custom code with verifed boot. This patch
> introduce new fastboot subcommand fastboot oem board:,
> which allow to run custom oem_board function.
>
> Default implementation is __weak. Vendor must redefine it in
> board/ folder with his own logic.
>
> For example, some vendors have their custom nand/emmc partition
> flashing or erasing. Here some typical command for such use cases:
>
> - flashing:
>
> $ fastboot stage bootloader.img
> $ fastboot oem board:write_bootloader
>
> - erasing:
>
> $ fastboot oem board:erase_env
>
> Signed-off-by: Alexey Romanov
> Reviewed-by: Mattijs Korpershoek
After applying this patch on master, it seems that the CI broke:
Building current source for 1 boards (1 thread, 64 jobs per thread)
sandbox: + sandbox64
+drivers/fastboot/fb_command.c: In function ‘oem_board’:
+drivers/fastboot/fb_command.c:580:43: error: passing argument 2 of
‘fastboot_oem_board’ makes pointer from integer without a cast
[-Werror=int-conversion]
+ 580 | fastboot_oem_board(cmd_parameter, fastboot_buf_addr,
image_size, response);
+ | ^
+ | |
+ | ulong {aka long unsigned int}
+drivers/fastboot/fb_command.c:567:59: note: expected ‘void *’ but argument is
of type ‘ulong’ {aka ‘long unsigned int’}
+ 567 | void __weak fastboot_oem_board(char *cmd_parameter, void *data, u32
size, char *response)
+ | ~~^~~~
+cc1: all warnings being treated as errors
+make[3]: *** [scripts/Makefile.build:256: drivers/fastboot/fb_command.o] Error
1
+make[2]: *** [scripts/Makefile.build:397: drivers/fastboot] Error 2
+make[1]: *** [Makefile:1892: drivers] Error 2
+make: *** [Makefile:177: sub-make] Error 2
001 /1 sandbox64
Completed: 1 total built, 1 newly), duration 0:00:08, rate 0.12
See:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/20398
Could you please have a look?
If you fix it, can you please send another version on top of master?
I will drop v4 and apply v5.
Thanks !
> ---
> doc/android/fastboot.rst | 18 ++
> drivers/fastboot/Kconfig | 7 +++
> drivers/fastboot/fb_command.c | 30 ++
> include/fastboot.h| 1 +
> 4 files changed, 56 insertions(+)
>
> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
> index 1ad8a897c8..2a627f9890 100644
> --- a/doc/android/fastboot.rst
> +++ b/doc/android/fastboot.rst
> @@ -29,6 +29,7 @@ The following OEM commands are supported (if enabled):
>with = boot_ack boot_partition
> - ``oem bootbus`` - this executes ``mmc bootbus %x %s`` to configure eMMC
> - ``oem run`` - this executes an arbitrary U-Boot command
> +- ``oem board`` - this executes a custom board function which is defined by
> the vendor
>
> Support for both eMMC and NAND devices is included.
>
> @@ -245,6 +246,23 @@ including multiple commands (using e.g. ``;`` or ``&&``)
> and control structures
> (``if``, ``while``, etc.). The exit code of ``fastboot`` will reflect the
> exit
> code of the command you ran.
>
> +Running Custom Vendor Code
> +^^
> +
> +U-Boot allows you to execute custom fastboot logic, which can be defined
> +in board/ files. It can still be used for production devices with verified
> +boot, because the vendor define logic at compile time by implementing
> +fastboot_oem_board() function. The attacker will not be able to execute
> +custom commands / code. For example, this can be useful for custom flashing
> +or erasing protocols::
> +
> +$ fastboot stage bootloader.img
> +$ fastboot oem board:write_bootloader
> +
> +In this case, ``cmd_parameter`` argument of the function
> ``fastboot_oem_board()``
> +will contain string "write_bootloader" and ``data`` argument is a pointer to
> +fastboot input buffer, which contains the contents of bootloader.img file.
> +
> References
> --
>
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index a4313d60a9..4d94391a76 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -241,6 +241,13 @@ config FASTBOOT_OEM_RUN
> this feature if you are using verified boot, as it will allow an
> attacker to bypass any restrictions you have in place.
>
> +config FASTBOOT_OEM_BOARD
> + bool "Enable the 'oem board' command"
> + help
> + This extends the fastboo
Re: [PATCH] Kconfig: Remove all default n options
Hi Michal,
Thank you for the patch.
On lun., avril 15, 2024 at 10:20, Michal Simek wrote:
> default n doesn't need to be specified. It is default option anyway.
> Similar changes have been done by commit 18370f14975c ("Kconfig: Remove all
> default n/no options").
>
> Signed-off-by: Michal Simek
Reviewed-by: Mattijs Korpershoek
> ---
>
> boot/Kconfig | 1 -
> cmd/Kconfig | 2 --
> 2 files changed, 3 deletions(-)
>
> diff --git a/boot/Kconfig b/boot/Kconfig
> index 777e408e2438..bb61e8b24f1c 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -816,7 +816,6 @@ if MEASURED_BOOT
>
> config MEASURE_IGNORE_LOG
> bool "Ignore the existing event log"
> - default n
> help
> On platforms that use an event log memory region that persists
> through system resets and are the first stage bootloader, then
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 8eeb99eea5ed..6c8ad1b87e87 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -444,7 +444,6 @@ config CMD_ELF
>
> config CMD_ELF_FDT_SETUP
> bool "Flattened Device Tree setup in bootelf cmd"
> - default n
> depends on CMD_ELF
> select LIB_LIBFDT
> select LMB
> @@ -2573,7 +2572,6 @@ config CMD_CROS_EC
> config CMD_SCMI
> bool "Enable scmi command"
> depends on SCMI_FIRMWARE
> - default n
> help
> This command provides user interfaces to several SCMI (System
> Control and Management Interface) protocols available on Arm
> --
> 2.44.0
Re: [PATCH] usb: musb-new: sunxi: support usage with DM_USB_GADGET
Hi,
On ven., avril 12, 2024 at 18:54, Aren wrote:
> On Thu, Apr 11, 2024 at 04:46:17PM +1000, John Watts wrote:
>> On Sun, Dec 31, 2023 at 03:38:37PM -0500, Aren Moynihan wrote:
>> > Add support for building the sunxi-musb driver with DM_USB_GADGET
>> > including adding a separate IRQ handling function and registering the
>> > driver with the musb system differently.
>>
>> Hi there,
>>
>> Were you aware of this similar patch?
>>
>> https://lore.kernel.org/u-boot/20230608195631.55364-1-cfswo...@gmail.com/
>>
>> If not you might want to test it and compare it.
>>
>> John.
>
> I was not aware of that patch, it looks like it does the same thing as
> this one, but without some of the bugs. When comparing with it I
> realized that my dm_usb_gadget_handle_interrupts function is just plain
> wrong.
>
> I'll try to find time to test it soon.
Please keep me in the loop for both progress on this one and on testing:
https://lore.kernel.org/u-boot/20230608195631.55364-1-cfswo...@gmail.com/
>
> Thanks for the link
> - Aren
>
>> > The implementation of usb_gadget_register_driver from
>> > musb-new/musb_uboot.c only works when the gadget driver for the device
>> > has already been probed and has called musb_register. On the pinephone
>> > (using a allwinner a64 processor) this causes issues when trying to use
>> > usb gadget mode (such as from the ums command) and CONFIG_USB_ETHER is
>> > disabled.
>> >
>> > The implementation of usb_gadget_register_driver provided when
>> > DM_USB_GADGET is enabled will probe the necessary drivers when it's
>> > called.
>> >
>> > Without the patch, this is what the error condition looks like:
>> > => ums 0 mmc 1
>> > UMS: LUN 0, dev mmc 1, hwpart 0, sector 0x0, count 0x3a3e000
>> > Controller uninitialized
>> > g_dnl_register: failed!, error: -6
>> > g_dnl_register failed
>> >
>> > based on:
>> > commit 2e4865bc6486 ("musb-new: omap2430: fix compiling in DM_USB_GADGET
>> > config")
>> >
>> > Signed-off-by: Aren Moynihan
Re: [PATCH v2 12/16] arm: dts: k3-am625-sk-u-boot: Add sysreset-controller node
Hi Jon,
On jeu., avril 11, 2024 at 15:30, Jon Humphreys wrote:
> Mattijs Korpershoek writes:
>
>> Hi Jonathan,
>>
>> Thank you for the patch.
>>
>> On lun., avril 08, 2024 at 17:31, Jonathan Humphreys
>> wrote:
>>
>>> Signed-off-by: Jonathan Humphreys
>>
>> Please consider adding a commit message body.
>
> Got it. thanks.
>
> BTW, the next version of this series will drop this patch as Andrew has
> submitted another patch removing the need for this one. See
> https://lore.kernel.org/r/20240402160908.508974-1-...@ti.com.
I was not aware of that one. thank you for pointing it out. That seems
indeed a better solution.
>
>>
>> On the TI vendor tree, there is a similar patch with a commit message:
>> https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2023.04=c5296d943c2c84dd6dcb3b91305d006ac46f3157
>>
>> Before patch:
>> => reset
>> resetting ...
>> System reset not supported on this platform
>> ### ERROR ### Please RESET the board ###
>>
>> With patch applied:
>> => reset
>> resetting ...
>>
>> Tested-by: Mattijs Korpershoek # on am62x sk evm
>>
>> Andrew also suggested to me that if we are interested by A53 reset only,
>> we can PSCI reset instead for all k3 architecture:
>>
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -784,6 +784,9 @@ config ARCH_K3
>> bool "Texas Instruments' K3 Architecture"
>> select SPL
>> select SUPPORT_SPL
>> + select PSCI_RESET if ARM64
>> + select SYSRESET if ARM64
>> + select SYSRESET_PSCI if ARM64
>> select FIT
>> select REGEX
>> select FIT_SIGNATURE if ARM64
>>
>> Has the above been considered?
>
> I am not aware. I would think that you want full reset, unless you are
> thinking about specifying a reset level?
On my end, I was mostly interested in resetting from the A53 core, but
the solution you linked previously should work for me.
>
> Jon
>
>>
>>
>>> ---
>>> arch/arm/dts/k3-am625-sk-u-boot.dtsi | 9 +
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
>>> b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
>>> index fa778b0ff4c..35bfeae75a0 100644
>>> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
>>> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
>>> @@ -46,3 +46,12 @@
>>> _port2 {
>>> status = "disabled";
>>> };
>>> +
>>> + {
>>> + bootph-pre-ram;
>>> +
>>> + k3_sysreset: sysreset-controller {
>>> + compatible = "ti,sci-sysreset";
>>> + bootph-pre-ram;
>>> + };
>>> +};
>>> --
>>> 2.34.1
Re: [PATCH] usb: dwc3: support USB 3.1 controllers
Hi Caleb,;
Thank you for the patch.
On jeu., avril 11, 2024 at 18:05, Caleb Connolly
wrote:
> The revision is different for these, add the additional check as in
> xhci-dwc3 core_init code.
>
> Signed-off-by: Caleb Connolly
Reviewed-by: Mattijs Korpershoek
> ---
> drivers/usb/dwc3/core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 96e850b7170f..db045f5822d4 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -594,9 +594,10 @@ static int dwc3_core_init(struct dwc3 *dwc)
> int ret;
>
> reg = dwc3_readl(dwc->regs, DWC3_GSNPSID);
> /* This should read as U3 followed by revision number */
> - if ((reg & DWC3_GSNPSID_MASK) != 0x5533) {
> + if ((reg & DWC3_GSNPSID_MASK) != 0x5533 &&
> + (reg & DWC3_GSNPSID_MASK) != 0x3331) {
> dev_err(dwc->dev, "this is not a DesignWare USB3 DRD Core\n");
> ret = -ENODEV;
> goto err0;
> }
> --
> 2.44.0
Re: [PATCH 2/2] disk: expose partition type flags
Hi Caleb,
Thank you for the patch.
On mar., avril 09, 2024 at 15:55, Caleb Connolly
wrote:
> GPT partition tables include two bytes worth of vendor defined
> attributes, per partition. ChromeOS and Qualcomm both use these (with
> different encoding!) to handle A/B slot switching with a retry counter.
>
> Expose these via the disk_partition struct so that they can be parsed by
> the relevant board code.
>
> This will be used on Qualcomm boards to determine which slot we're
> booting on so that we can flash capsule updates to the correct one.
>
> Signed-off-by: Caleb Connolly
Reviewed-by: Mattijs Korpershoek
> ---
> disk/part_efi.c | 1 +
> include/part.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index 4ce9243ef25c..d3ce4dd01dcd 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -292,8 +292,9 @@ int part_get_info_efi(struct blk_desc *desc, int part,
> snprintf((char *)info->name, sizeof(info->name), "%s",
>print_efiname(_pte[part - 1]));
> strcpy((char *)info->type, "U-Boot");
> info->bootable = get_bootable(_pte[part - 1]);
> + info->type_flags = gpt_pte[part -
> 1].attributes.fields.type_guid_specific;
> if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
> uuid_bin_to_str(gpt_pte[part - 1].unique_partition_guid.b,
> (char *)disk_partition_uuid(info),
> UUID_STR_FORMAT_GUID);
> diff --git a/include/part.h b/include/part.h
> index 32ee40488563..afae51f1b933 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -68,8 +68,9 @@ struct disk_partition {
>* PART_BOOTABLEthe MBR bootable flag is set
>* PART_EFI_SYSTEM_PARTITIONthe partition is an EFI system partition
>*/
> int bootable;
> + u16 type_flags; /* top 16 bits of GPT partition attributes
> */
> #if CONFIG_IS_ENABLED(PARTITION_UUIDS)
> charuuid[UUID_STR_LEN + 1]; /* filesystem UUID as string, if exists
> */
> #endif
> #ifdef CONFIG_PARTITION_TYPE_GUID
>
> --
> 2.44.0
Re: [PATCH 1/2] dfu: add scsi backend
Hi Caleb,
Thank you for the patch.
On mar., avril 09, 2024 at 15:55, Caleb Connolly
wrote:
> This is extremely similar to the MMC backend, but there are some notable
> differences.
>
> Works with a DFU string like
>
> scsi 4=u-boot-bin part 11
>
> Where "4" is the SCSI dev number (sequential LUN across all SCSI devices)
> and "11" is the partition number.
>
> Signed-off-by: Caleb Connolly
> ---
> doc/usage/dfu.rst | 31
> drivers/dfu/Kconfig| 7 +
> drivers/dfu/Makefile | 1 +
> drivers/dfu/dfu.c | 5 +-
> drivers/dfu/dfu_scsi.c | 437
> +
> include/dfu.h | 26 +++
> 6 files changed, 506 insertions(+), 1 deletion(-)
>
> diff --git a/doc/usage/dfu.rst b/doc/usage/dfu.rst
> index 8cc09c308d82..dc4f8d672f99 100644
> --- a/doc/usage/dfu.rst
> +++ b/doc/usage/dfu.rst
> @@ -166,8 +166,38 @@ mmc
Can we also add this to the summary list at line 22?
"Today the supported DFU backends are:"
Also, please add CONFIG_DFU_SCSI in "Configuration Options" section at
the beginning of this document.
>
> Please note that this means the user will be able to execute any
> arbitrary commands just like in the u-boot's shell.
>
> +scsi
> +for UFS storage::
> +
> +dfu 0 scsi
> +
> +each element in *dfu_alt_info* being
> +
> +* raw raw access to SCSI LUN
> +* part raw access to partition
> +* fatfile in FAT partition
> +* ext4 file in EXT4 partition
> +* skip 0 0ignore flashed data
> +* script 0 0 execute commands in shell
> +
> +with
> +
> +size
> +is the size of the access area (hexadecimal without "0x")
> +or 0 which means whole device
> +partid
> +is the GPT or DOS partition index.
> +dev
> +is the SCSI LU (Logical Unit) index (decimal only)
> +
> +A value of environment variable *dfu_alt_info* for UFS could be::
> +
> +u-boot part 4;bl2 raw 0x1e 0x1d
> +
> +See mmc section above for details on the skip and script types.
> +
> nand
> raw slc nand device::
>
> dfu 0 nand
> @@ -277,8 +307,9 @@ alternate list separated by '&' with the same format for
> each ::
>
> mmc =;;
> nand =;;
> ram =;;
> +scsi =;;
> sf =;;
> mtd =;;
> virt =;;
>
> diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> index 0360d9da1427..158c660e6c4e 100644
> --- a/drivers/dfu/Kconfig
> +++ b/drivers/dfu/Kconfig
> @@ -86,8 +86,15 @@ config DFU_VIRT
> This option enables using DFU to read and write to VIRTUAL device
> used at board level to manage specific behavior
> (OTP update for example).
>
> +config DFU_SCSI
> + bool "SCSI flash back end for DFU"
> + help
> + This option enables using DFU to read and write to SCSI devices
> + used at board level to manage specific behavior
> + (OTP update for example).
> +
> config SET_DFU_ALT_INFO
> bool "Dynamic set of DFU alternate information"
> help
> This option allows to call the function set_dfu_alt_info to
> diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile
> index dfbf64da6677..3b3ba0994b3a 100644
> --- a/drivers/dfu/Makefile
> +++ b/drivers/dfu/Makefile
> @@ -10,4 +10,5 @@ obj-$(CONFIG_$(SPL_)DFU_NAND) += dfu_nand.o
> obj-$(CONFIG_$(SPL_)DFU_RAM) += dfu_ram.o
> obj-$(CONFIG_$(SPL_)DFU_SF) += dfu_sf.o
> obj-$(CONFIG_$(SPL_)DFU_WRITE_ALT) += dfu_alt.o
> obj-$(CONFIG_$(SPL_)DFU_VIRT) += dfu_virt.o
> +obj-$(CONFIG_$(SPL_)DFU_SCSI) += dfu_scsi.o
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 2adf26e2fe24..8024ad98a7dc 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -548,8 +548,11 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char
> *s, int alt,
> return -1;
> } else if (strcmp(interface, "virt") == 0) {
> if (dfu_fill_entity_virt(dfu, devstr, argv, argc))
> return -1;
> + } else if (strcmp(interface, "scsi") == 0) {
> + if (dfu_fill_entity_scsi(dfu, devstr, argv, argc))
> + return -1;
> } else {
> printf("%s: Device %s not (yet) supported!\n",
> __func__, interface);
> return -1;
> @@ -644,9 +647,9 @@ int dfu_config_entities(char *env, char *interface, char
> *devstr)
>
> const char *dfu_get_dev_type(enum dfu_device_type t)
> {
> const char *const dev_t[] = {NULL, "eMMC", "OneNAND", "NAND", "RAM",
> - "SF", "MTD", "VIRT"};
> + "SF", "MTD", "VIRT", "SCSI"};
> return dev_t[t];
> }
>
> const char *dfu_get_layout(enum dfu_layout l)
> diff --git a/drivers/dfu/dfu_scsi.c b/drivers/dfu/dfu_scsi.c
> new file mode 100644
> index ..63c3bcffe769
> --- /dev/null
> +++
Re: [PATCH] usb: dwc2: Add in version 4xx compatibility
Hi Greg,
Thank you for the contribution.
On mar., mars 26, 2024 at 11:36, Greg Malysa wrote:
Please avoid top-posting when replying, it makes following the
discussion more difficult:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions
> I'd be happy with that change. Does anyone have access to the
> associated designware databook (I do not)? We could also check to see
> if those four bits are all always allocated to the 2/3/4/x version
> number. I can submit v2 with that change instead once we know.
I don't have access to the databooks either. I usually refer to the
linux kernel code since it's a more up to date version of this driver.
Looking at
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c
we can see that the following is added:
#define DWC2_CORE_REV_MASK 0x
This makes me believe that the versioning follows a well known pattern.
>
> On Tue, Mar 26, 2024 at 7:50 AM Marek Vasut wrote:
>>
>> On 3/26/24 3:32 AM, Greg Malysa wrote:
>> > From: Nathan Barrett-Morrison
>> >
>> > This adds the Synopsys device id for version 4xx of the designware
>> > IP block and extends the version check to include it to permit
>> > new hardware to run. It does not add any 4xx-specific features.
>> >
>> > Signed-off-by: Ian Roberts
>> > Signed-off-by: Greg Malysa
>> > Signed-off-by: Nathan Barrett-Morrison
>> >
>> > ---
>> >
>> >
>> > ---
>> > drivers/usb/host/dwc2.c | 3 ++-
>> > drivers/usb/host/dwc2.h | 1 +
>> > 2 files changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
>> > index 637eb2dd06..6fdde6a9a7 100644
>> > --- a/drivers/usb/host/dwc2.c
>> > +++ b/drivers/usb/host/dwc2.c
>> > @@ -1180,7 +1180,8 @@ static int dwc2_init_common(struct udevice *dev,
>> > struct dwc2_priv *priv)
>> >snpsid >> 12 & 0xf, snpsid & 0xfff);
>> >
>> > if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&
>> > - (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {
>> > + (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx &&
>> > + (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_4xx) {
Note that this change is also part of:
https://lore.kernel.org/all/20240328131811.94559-1-seashell11234...@gmail.com/
>> > dev_info(dev, "SNPSID invalid (not DWC2 OTG device): %08x\n",
>> >snpsid);
>> > return -ENODEV;
>> > diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h
>> > index 6f022e33a1..f202d55eb2 100644
>> > --- a/drivers/usb/host/dwc2.h
>> > +++ b/drivers/usb/host/dwc2.h
>> > @@ -739,6 +739,7 @@ struct dwc2_core_regs {
>> > #define DWC2_PCGCCTL_DEEP_SLEEP_OFFSET 7
>> > #define DWC2_SNPSID_DEVID_VER_2xx (0x4f542 << 12)
>> > #define DWC2_SNPSID_DEVID_VER_3xx (0x4f543 << 12)
>> > +#define DWC2_SNPSID_DEVID_VER_4xx(0x4f544 << 12)
>> > #define DWC2_SNPSID_DEVID_MASK (0xf <<
>> > 12)
>> > #define DWC2_SNPSID_DEVID_OFFSET12
>>
>> Maybe it would be better/easier/futureproof to simply check if (snpsid &
>> 0x0 == 0x4f540) ?
>
>
>
> --
> Greg Malysa
> Timesys Corporation
Re: [PATCH] usb: dwc2: update reset method for host and device mode
On mar., avril 16, 2024 at 10:50, Mattijs Korpershoek
wrote:
> Hi Kongyang,
>
> Thank you for the patch.
>
> On jeu., mars 28, 2024 at 21:14, Kongyang Liu
> wrote:
>
>> Starting from version 4.20a, there has been a change in the reset method.
>> A new bit, GRSTCTL_CSFTRST_DONE, has been introduced in the GRSTCTL
>> register to indicate whether the reset has been completed.
>>
>> Signed-off-by: Kongyang Liu
>
> I've compared this with the equivalent Linux patch found here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c
>
> Reviewed-by: Mattijs Korpershoek
>
> Also tested on VIM3 that I could use fastboot.
>
> Tested-by: Mattijs Korpershoek # on vim3
>
> I will add the linux link in the commit message when applying. Please
> consider adding it yourself if you receive review comments that require
> sending a v2.
Just noticed that this also touches the host part of the driver.
Marek, do you want to review/pick this up, or can this go through
u-boot-dfu ?
>
> Thanks
> Mattijs
>
>> ---
>>
>> drivers/usb/gadget/dwc2_udc_otg.c | 18 --
>> drivers/usb/gadget/dwc2_udc_otg_regs.h | 19 +--
>> drivers/usb/host/dwc2.c| 19 ---
>> drivers/usb/host/dwc2.h| 6 ++
>> 4 files changed, 51 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c
>> b/drivers/usb/gadget/dwc2_udc_otg.c
>> index 27082f5152..d1dd469a0f 100644
>> --- a/drivers/usb/gadget/dwc2_udc_otg.c
>> +++ b/drivers/usb/gadget/dwc2_udc_otg.c
>> @@ -42,7 +42,7 @@
>> #include
>> #include
>>
>> -#include
>> +#include
>>
>> #include
>>
>> @@ -464,12 +464,26 @@ static void reconfig_usbd(struct dwc2_udc *dev)
>> {
>> /* 2. Soft-reset OTG Core and then unreset again. */
>> int i;
>> -unsigned int uTemp = writel(CORE_SOFT_RESET, >grstctl);
>> +unsigned int uTemp;
>> uint32_t dflt_gusbcfg;
>> uint32_t rx_fifo_sz, tx_fifo_sz, np_tx_fifo_sz;
>> u32 max_hw_ep;
>> int pdata_hw_ep;
>>
>> +u32 snpsid, greset;
>> +
>> +snpsid = readl(>gsnpsid);
>> +writel(CORE_SOFT_RESET, >grstctl);
>> +if ((snpsid & SNPSID_VER_MASK) < (SNPSID_VER_420a & SNPSID_VER_MASK)) {
>> +wait_for_bit_le32(>grstctl, CORE_SOFT_RESET, false, 1000,
>> false);
>> +} else {
>> +wait_for_bit_le32(>grstctl, CORE_SOFT_RESET_DONE, true,
>> 1000, false);
>> +greset = readl(>grstctl);
>> +greset &= ~CORE_SOFT_RESET;
>> +greset |= CORE_SOFT_RESET_DONE;
>> +writel(greset, >grstctl);
>> +}
>> +
>> debug("Resetting OTG controller\n");
>>
>> dflt_gusbcfg =
>> diff --git a/drivers/usb/gadget/dwc2_udc_otg_regs.h
>> b/drivers/usb/gadget/dwc2_udc_otg_regs.h
>> index 9ca6f42375..b3d9117033 100644
>> --- a/drivers/usb/gadget/dwc2_udc_otg_regs.h
>> +++ b/drivers/usb/gadget/dwc2_udc_otg_regs.h
>> @@ -63,24 +63,26 @@ struct dwc2_usbotg_reg {
>> u32 gnptxfsiz; /* Non-Periodic Transmit FIFO Size */
>> u8 res0[12];
>> u32 ggpio; /* 0x038 */
>> -u8 res1[20];
>> +u8 res1[4];
>> +u32 gsnpsid;
>> +u8 res2[12];
>> u32 ghwcfg4; /* User HW Config4 */
>> -u8 res2[176];
>> +u8 res3[176];
>> u32 dieptxf[15]; /* Device Periodic Transmit FIFO size register */
>> -u8 res3[1728];
>> +u8 res4[1728];
>> /* Device Configuration */
>> u32 dcfg; /* Device Configuration Register */
>> u32 dctl; /* Device Control */
>> u32 dsts; /* Device Status */
>> -u8 res4[4];
>> +u8 res5[4];
>> u32 diepmsk; /* Device IN Endpoint Common Interrupt Mask */
>> u32 doepmsk; /* Device OUT Endpoint Common Interrupt Mask */
>> u32 daint; /* Device All Endpoints Interrupt */
>> u32 daintmsk; /* Device All Endpoints Interrupt Mask */
>> -u8 res5[224];
>> +u8 res6[224];
>> struct dwc2_dev_in_endp in_endp[16];
>> struct dwc2_dev_out_endp out_endp[16];
>> -u8 res6[768];
>> +u8 res7[768];
>> struct ep_fifo ep[16];
>> };
>>
>> @@ -118,6 +120,7 @@ struct dwc2_usbotg_reg {
>> /* DWC2_UDC_OTG_GRSTCTL */
>> #define AHB_MASTER_IDLE (1u<<31)
>>
Re: [PATCH] usb: dwc2: update reset method for host and device mode
Hi Kongyang,
Thank you for the patch.
On jeu., mars 28, 2024 at 21:14, Kongyang Liu
wrote:
> Starting from version 4.20a, there has been a change in the reset method.
> A new bit, GRSTCTL_CSFTRST_DONE, has been introduced in the GRSTCTL
> register to indicate whether the reset has been completed.
>
> Signed-off-by: Kongyang Liu
I've compared this with the equivalent Linux patch found here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c
Reviewed-by: Mattijs Korpershoek
Also tested on VIM3 that I could use fastboot.
Tested-by: Mattijs Korpershoek # on vim3
I will add the linux link in the commit message when applying. Please
consider adding it yourself if you receive review comments that require
sending a v2.
Thanks
Mattijs
> ---
>
> drivers/usb/gadget/dwc2_udc_otg.c | 18 --
> drivers/usb/gadget/dwc2_udc_otg_regs.h | 19 +--
> drivers/usb/host/dwc2.c| 19 ---
> drivers/usb/host/dwc2.h| 6 ++
> 4 files changed, 51 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c
> b/drivers/usb/gadget/dwc2_udc_otg.c
> index 27082f5152..d1dd469a0f 100644
> --- a/drivers/usb/gadget/dwc2_udc_otg.c
> +++ b/drivers/usb/gadget/dwc2_udc_otg.c
> @@ -42,7 +42,7 @@
> #include
> #include
>
> -#include
> +#include
>
> #include
>
> @@ -464,12 +464,26 @@ static void reconfig_usbd(struct dwc2_udc *dev)
> {
> /* 2. Soft-reset OTG Core and then unreset again. */
> int i;
> - unsigned int uTemp = writel(CORE_SOFT_RESET, >grstctl);
> + unsigned int uTemp;
> uint32_t dflt_gusbcfg;
> uint32_t rx_fifo_sz, tx_fifo_sz, np_tx_fifo_sz;
> u32 max_hw_ep;
> int pdata_hw_ep;
>
> + u32 snpsid, greset;
> +
> + snpsid = readl(>gsnpsid);
> + writel(CORE_SOFT_RESET, >grstctl);
> + if ((snpsid & SNPSID_VER_MASK) < (SNPSID_VER_420a & SNPSID_VER_MASK)) {
> + wait_for_bit_le32(>grstctl, CORE_SOFT_RESET, false, 1000,
> false);
> + } else {
> + wait_for_bit_le32(>grstctl, CORE_SOFT_RESET_DONE, true,
> 1000, false);
> + greset = readl(>grstctl);
> + greset &= ~CORE_SOFT_RESET;
> + greset |= CORE_SOFT_RESET_DONE;
> + writel(greset, >grstctl);
> + }
> +
> debug("Resetting OTG controller\n");
>
> dflt_gusbcfg =
> diff --git a/drivers/usb/gadget/dwc2_udc_otg_regs.h
> b/drivers/usb/gadget/dwc2_udc_otg_regs.h
> index 9ca6f42375..b3d9117033 100644
> --- a/drivers/usb/gadget/dwc2_udc_otg_regs.h
> +++ b/drivers/usb/gadget/dwc2_udc_otg_regs.h
> @@ -63,24 +63,26 @@ struct dwc2_usbotg_reg {
> u32 gnptxfsiz; /* Non-Periodic Transmit FIFO Size */
> u8 res0[12];
> u32 ggpio; /* 0x038 */
> - u8 res1[20];
> + u8 res1[4];
> + u32 gsnpsid;
> + u8 res2[12];
> u32 ghwcfg4; /* User HW Config4 */
> - u8 res2[176];
> + u8 res3[176];
> u32 dieptxf[15]; /* Device Periodic Transmit FIFO size register */
> - u8 res3[1728];
> + u8 res4[1728];
> /* Device Configuration */
> u32 dcfg; /* Device Configuration Register */
> u32 dctl; /* Device Control */
> u32 dsts; /* Device Status */
> - u8 res4[4];
> + u8 res5[4];
> u32 diepmsk; /* Device IN Endpoint Common Interrupt Mask */
> u32 doepmsk; /* Device OUT Endpoint Common Interrupt Mask */
> u32 daint; /* Device All Endpoints Interrupt */
> u32 daintmsk; /* Device All Endpoints Interrupt Mask */
> - u8 res5[224];
> + u8 res6[224];
> struct dwc2_dev_in_endp in_endp[16];
> struct dwc2_dev_out_endp out_endp[16];
> - u8 res6[768];
> + u8 res7[768];
> struct ep_fifo ep[16];
> };
>
> @@ -118,6 +120,7 @@ struct dwc2_usbotg_reg {
> /* DWC2_UDC_OTG_GRSTCTL */
> #define AHB_MASTER_IDLE (1u<<31)
> #define CORE_SOFT_RESET (0x1<<0)
> +#define CORE_SOFT_RESET_DONE (0x1<<29)
>
> /* DWC2_UDC_OTG_GINTSTS/DWC2_UDC_OTG_GINTMSK core interrupt register */
> #define INT_RESUME (1u<<31)
> @@ -285,6 +288,10 @@ struct dwc2_usbotg_reg {
> #define DAINT_IN_EP_INT(x)(x << 0)
> #define DAINT_OUT_EP_INT(x) (x << 16)
>
> +/* DWC2_UDC_OTG_GSNPSID */
> +#define SNPSID_VER_420a 0x4f54420a
> +#define SNPSID_VER_MASK 0x
> +
> /* Use
Re: [PATCH v3 0/1] Introduce fastboot oem board command
Hi, On Mon, 08 Apr 2024 13:15:51 +0300, Alexey Romanov wrote: > Changes V1 -> V2 [1]: > - Added an example of using the command as requested > by Sean Anderson [2]. > > Changes V2 -> V3 [3]: > - Rebase over uboot/master. > - Add documentation. > - Remove example added in V2 [1]. > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next) [1/1] fastboot: introduce 'oem board' subcommand https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/78ae23de701bd65501079b4c39158f152bfbafa9 -- Mattijs
Re: [PATCH v2 12/16] arm: dts: k3-am625-sk-u-boot: Add sysreset-controller node
Hi Jonathan,
Thank you for the patch.
On lun., avril 08, 2024 at 17:31, Jonathan Humphreys wrote:
> Signed-off-by: Jonathan Humphreys
Please consider adding a commit message body.
On the TI vendor tree, there is a similar patch with a commit message:
https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2023.04=c5296d943c2c84dd6dcb3b91305d006ac46f3157
Before patch:
=> reset
resetting ...
System reset not supported on this platform
### ERROR ### Please RESET the board ###
With patch applied:
=> reset
resetting ...
Tested-by: Mattijs Korpershoek # on am62x sk evm
Andrew also suggested to me that if we are interested by A53 reset only,
we can PSCI reset instead for all k3 architecture:
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -784,6 +784,9 @@ config ARCH_K3
bool "Texas Instruments' K3 Architecture"
select SPL
select SUPPORT_SPL
+ select PSCI_RESET if ARM64
+ select SYSRESET if ARM64
+ select SYSRESET_PSCI if ARM64
select FIT
select REGEX
select FIT_SIGNATURE if ARM64
Has the above been considered?
> ---
> arch/arm/dts/k3-am625-sk-u-boot.dtsi | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> index fa778b0ff4c..35bfeae75a0 100644
> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> @@ -46,3 +46,12 @@
> _port2 {
> status = "disabled";
> };
> +
> + {
> + bootph-pre-ram;
> +
> + k3_sysreset: sysreset-controller {
> + compatible = "ti,sci-sysreset";
> + bootph-pre-ram;
> + };
> +};
> --
> 2.34.1
Re: [PATCH v4 1/1] fastboot: introduce 'oem board' subcommand
Hi Alexey, On mer., avril 10, 2024 at 12:43, Alexey Romanov wrote: > Hi guys, > > On Wed, Apr 10, 2024 at 02:02:21PM +0200, Quentin Schulz wrote: >> Hi Alexey, >> >> On 4/10/24 12:58, Alexey Romanov wrote: >> > Currently, fastboot protocol in U-Boot has no opportunity >> > to execute vendor custom code with verifed boot. This patch >> > introduce new fastboot subcommand fastboot oem board:, >> > which allow to run custom oem_board function. >> > >> > Default implementation is __weak. Vendor must redefine it in >> > board/ folder with his own logic. >> > >> > For example, some vendors have their custom nand/emmc partition >> > flashing or erasing. Here some typical command for such use cases: >> > >> > - flashing: >> > >> >$ fastboot stage bootloader.img >> >$ fastboot oem board:write_bootloader >> > >> > - erasing: >> > >> >$ fastboot oem board:erase_env >> > >> > Signed-off-by: Alexey Romanov >> > Reviewed-by: Mattijs Korpershoek >> > --- >> > doc/android/fastboot.rst | 18 ++ >> > drivers/fastboot/Kconfig | 7 +++ >> > drivers/fastboot/fb_command.c | 30 ++ >> > include/fastboot.h| 1 + >> > 4 files changed, 56 insertions(+) >> > >> > diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst >> > index 1ad8a897c8..2a627f9890 100644 >> > --- a/doc/android/fastboot.rst >> > +++ b/doc/android/fastboot.rst >> > @@ -29,6 +29,7 @@ The following OEM commands are supported (if enabled): >> > with = boot_ack boot_partition >> > - ``oem bootbus`` - this executes ``mmc bootbus %x %s`` to configure >> > eMMC >> > - ``oem run`` - this executes an arbitrary U-Boot command >> > +- ``oem board`` - this executes a custom board function which is defined >> > by the vendor >> > Support for both eMMC and NAND devices is included. >> > @@ -245,6 +246,23 @@ including multiple commands (using e.g. ``;`` or >> > ``&&``) and control structures >> > (``if``, ``while``, etc.). The exit code of ``fastboot`` will reflect >> > the exit >> > code of the command you ran. >> > +Running Custom Vendor Code >> > +^^ >> > + >> > +U-Boot allows you to execute custom fastboot logic, which can be defined >> > +in board/ files. It can still be used for production devices with verified >> > +boot, because the vendor define logic at compile time by implementing >> >> I think it should be "defines" here? > > Yep. If there are no more comments, maybe Mattijs will correct this when > he picks up a patch? So that I don't send a new series with typo fix :) Yes, if there are no other review comments, I will fix this up locally when picking this up. > >> >> Reviewed-by: Quentin Schulz >> >> Thanks, >> Quentin > > -- > Thank you, > Alexey
Re: [PATCH] doc: build: update docker image to latest
Hi Tom,
Thank you for the review.
On lun., avril 08, 2024 at 14:11, Tom Rini wrote:
> On Fri, Apr 05, 2024 at 08:16:19AM +0200, Mattijs Korpershoek wrote:
>
>
>> commit cacc0b2678c0 ("CI: Move to latest container image") updated the
>> docker container image.
>>
>> Reflect the change in the documentation.
>>
>> Signed-off-by: Mattijs Korpershoek
>
> I'm fine with updating this for now, so:
>
> Reviewed-by: Tom Rini
>
> But I think longer term we/I should reword the section to be clear
> that's an example syntax not a "recreate the current tag" since only I
> can push that result.
I agree. I will do another reword later to explain where to find the
reference image (pointing to .gitlab-ci.yml)
>
> --
> Tom
Re: [PATCH v3 1/1] fastboot: introduce 'oem board' subcommand
Hi Alexey,
Thank you for the patch.
On lun., avril 08, 2024 at 13:15, Alexey Romanov
wrote:
> Currently, fastboot protocol in U-Boot has no opportunity
> to execute vendor custom code with verifed boot. This patch
> introduce new fastboot subcommand fastboot oem board:,
> which allow to run custom oem_board function.
>
> Default implementation is __weak. Vendor must redefine it in
> board/ folder with his own logic.
>
> For example, some vendors have their custom nand/emmc partition
> flashing or erasing. Here some typical command for such use cases:
>
> - flashing:
>
> $ fastboot stage bootloader.img
> $ fastboot oem board:write_bootloader
>
> - erasing:
>
> $ fastboot oem board:erase_env
>
> Signed-off-by: Alexey Romanov
Reviewed-by: Mattijs Korpershoek
Thank you for being patient on this topic!
I'll be awaiting 2 more days and will apply to the u-boot-dfu if no
other remarks have been made.
> ---
> doc/android/fastboot.rst | 18 ++
> drivers/fastboot/Kconfig | 7 +++
> drivers/fastboot/fb_command.c | 30 ++
> include/fastboot.h| 1 +
> 4 files changed, 56 insertions(+)
>
> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
> index 05d8f77759..2020590657 100644
> --- a/doc/android/fastboot.rst
> +++ b/doc/android/fastboot.rst
> @@ -30,6 +30,7 @@ The following OEM commands are supported (if enabled):
> - ``oem bootbus`` - this executes ``mmc bootbus %x %s`` to configure eMMC
> - ``oem run`` - this executes an arbitrary U-Boot command
> - ``oem console`` - this dumps U-Boot console record buffer
> +- ``oem board`` - this executes an custom board function which is defined by
> vendor
>
> Support for both eMMC and NAND devices is included.
>
> @@ -246,6 +247,23 @@ including multiple commands (using e.g. ``;`` or ``&&``)
> and control structures
> (``if``, ``while``, etc.). The exit code of ``fastboot`` will reflect the
> exit
> code of the command you ran.
>
> +Running Custom Vendor Code
> +^^
> +
> +U-Boot allows you to execute custom fastboot logic, which can be defined
> +in board/ files. It can still be used for production devices with verified
> +boot, because vendor define logic at compile time by overriding weak
> +implementation of fastboot_oem_board() function. The attacker will
> +not able to execute his commands / code. For example, this can be useful
> +for custom flashing or erasing protocols::
> +
> +$ fastboot stage bootloader.img
> +$ fastboot oem board:write_bootloader
> +
> +In this case, ``cmd_parameter`` argument of the function
> ``fastboot_oem_board()``
> +will contain string "write_bootloader" and ``data`` argument is a pointer to
> +fastboot input buffer, which containing the contents of bootloader.img file.
> +
> References
> --
>
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 5e5855a76c..937a39f54a 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -249,6 +249,13 @@ config FASTBOOT_CMD_OEM_CONSOLE
> Add support for the "oem console" command to input and read console
> record buffer.
>
> +config FASTBOOT_OEM_BOARD
> + bool "Enable the 'oem board' command"
> + help
> + This extends the fastboot protocol with an "oem board" command. This
> + command allows running vendor custom code defined in board/ files.
> + Otherwise, it will do nothing and send fastboot fail.
> +
> endif # FASTBOOT
>
> endmenu
> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
> index f95f4e4ae1..96c27afc60 100644
> --- a/drivers/fastboot/fb_command.c
> +++ b/drivers/fastboot/fb_command.c
> @@ -42,6 +42,7 @@ static void oem_format(char *, char *);
> static void oem_partconf(char *, char *);
> static void oem_bootbus(char *, char *);
> static void oem_console(char *, char *);
> +static void oem_board(char *, char *);
> static void run_ucmd(char *, char *);
> static void run_acmd(char *, char *);
>
> @@ -113,6 +114,10 @@ static const struct {
> .command = "oem console",
> .dispatch = CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_CONSOLE,
> (oem_console), (NULL))
> },
> + [FASTBOOT_COMMAND_OEM_BOARD] = {
> + .command = "oem board",
> + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_BOARD, (oem_board),
> (NULL))
> + },
> [FASTBOOT_COMMAND_UCMD] = {
> .command = "UCmd",
> .dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run
Re: [RFC PATCH v2 2/2] board: ad401: example of fastboot oem board realization
Hi Alexey,
On lun., mars 04, 2024 at 14:03, Alexey Romanov
wrote:
> Hello Mattijs,
>
> On Thu, Feb 15, 2024 at 10:24:11AM +0100, Mattijs Korpershoek wrote:
>> On jeu., f'evr. 01, 2024 at 12:20, Alexey Romanov
>> wrote:
>>
>> > An example of how we use fastboot oeam board subcommand
>> > for Sean Anderson.
>> >
>> > 1 - OEM_BOARD_WRITE_BOOTLOADER_CMD:
>> >
>> > We use it for custom Amlogic bootloader + tpl
>> > flashing protocol.
>> >
>> > 2 - OEM_BOARD_ERASE_CMD:
>> >
>> > Custom logic for erasing the env-emulated partition,
>> > which isn't in the mtd markup map.
>> >
>> > Example of the script which completely flashes the device:
>> >
>> > $ fastboot erase bootloader
>> > $ fastboot stage u-boot.bin
>> > $ fastboot oem board:write_bootloader
>> > $ fastboot reboot-bootloader
>> > $ fastboot oem board:erase_env
>> > $ fastboot erase misc
>> > $ fastboot erase super
>> > $ fastboot flash super rootfs
>> > $ fastboot reboot
>> >
>> > Signed-off-by: Alexey Romanov
>> > ---
>> > board/amlogic/ad401/fastboot.c | 222 +
>> > 1 file changed, 222 insertions(+)
>> > create mode 100644 board/amlogic/ad401/fastboot.c
>> >
>> > diff --git a/board/amlogic/ad401/fastboot.c
>> > b/board/amlogic/ad401/fastboot.c
>> > new file mode 100644
>> > index 00..01da8efa5b
>> > --- /dev/null
>> > +++ b/board/amlogic/ad401/fastboot.c
>> > @@ -0,0 +1,222 @@
>> > +// SPDX-License-Identifier: GPL-2.0+
>> > +/*
>> > + * (C) Copyright 2023 SaluteDevices, Inc.
>> > + */
>> > +
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +#include
>> > +
>> > +enum {
>> > + OEM_BOARD_ERASE_CMD,
>> > + OEM_BOARD_WRITE_BOOTLOADER_CMD,
>> > +};
>> > +
>> > +struct defenv {
>> > + char *name;
>> > + char value[256];
>> > +};
>> > +
>> > +static void save_defenv(struct defenv *e, size_t cnt)
>> > +{
>> > + int i;
>> > +
>> > + for (i = 0; i < cnt; i++) {
>> > + const char *env_val = env_get(e[i].name);
>> > +
>> > + if (env_val)
>> > + strlcpy(e[i].value, env_val, sizeof(e[i].value));
>> > + else
>> > + e[i].value[0] = '\0';
>> > + }
>> > +}
>> > +
>> > +static void set_defenv(struct defenv *e, size_t cnt)
>> > +{
>> > + int i;
>> > +
>> > + for (i = 0; i < cnt; i++)
>> > + env_set(e[i].name, e[i].value);
>> > +}
>> > +
>> > +static int fastboot_erase_env(void)
>> > +{
>> > + char *const defenv_names[] = { "lock", "mtdparts", "mtdids" };
>> > + struct defenv env[ARRAY_SIZE(defenv_names)];
>> > + int err, i;
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(env); i++)
>> > + env[i].name = defenv_names[i];
>> > +
>> > + printf("ENV is being erased...\n");
>> > +
>> > + /*
>> > + * Reset environment to the default, excluding 'lock' variable,
>> > + * because it reflects the fastboot's state after execution of
>> > + * 'flashing unlock' command, hence it must survive the env-erasing.
>> > + * Otherwise, further erase commands will fail on check_lock().
>> > + *
>> > + * Also, we have to save 'mtdparts' and 'mtdids' variables
>> > + * because they are necessary to obtain partition map.
>> > + */
>> > +
>> > + save_defenv(env, ARRAY_SIZE(env));
>> > + env_set_default(NULL, 0);
>> > + set_defenv(env, ARRAY_SIZE(env));
>> > +
>> > + err = env_save();
>> > + if (err) {
>> > + pr_err("Can't overwrite ENV-partition\n");
>> > + return err;
>> > + }
>>
>> Hmm so the fastboot locked state is saved in the U-Boot environment.
>> There is probably a good reason for this (no secure storage for
>> example). But this does not feel board specific.
>>
>> Wouldn't it be better if we could just run "fastboot erase bootenv" and
>&g
Re: [RFC PATCH v2 1/2] fastboot: introduce 'oem board' subcommand
Hi Alexey, On lun., mars 04, 2024 at 14:11, Alexey Romanov wrote: > Hello, > > On Thu, Feb 15, 2024 at 10:14:13AM +0100, Mattijs Korpershoek wrote: >> Hi Alexey, >> >> Thank you for the patch. >> >> On jeu., f'evr. 01, 2024 at 12:20, Alexey Romanov >> wrote: >> >> > Currently, fastboot protocol in U-Boot has no opportunity >> > to execute vendor custom code with verifed boot. This patch >> > introduce new fastboot subcommand fastboot oem board:, >> > which allow to run custom oem_board function. >> > >> > Default implementation is __weak. Vendor must redefine it in >> > board/ folder with his own logic. >> > >> > For example, some vendors have their custom nand/emmc partition >> > flashing or erasing. Here some typical command for such use cases: >> > >> > - flashing: >> > >> > $ fastboot stage bootloader.img >> > $ fastboot oem board:write_bootloader >> > >> > - erasing: >> > >> > $ fastboot oem board:erase_env >> > >> > Signed-off-by: Alexey Romanov >> >> Sorry for the delay. I needed time to give this some thoughts and I >> waited for Sean to chime as well on this. >> >> I've heard from Neil that this might be related to: >> https://github.com/superna/pyamlboot/pull/20 > > Yeah, pyamlboot also uses the same 'bootloader' partition flashing > scheme as I present in the patch 2. This is custom Amlogic protocol. > >> >> I think this can be useful. Not necessarily for writing custom >> partitions, but I see this could be used for other things: >> >> 1. Provision SoC-specific fuses (serialno/mac addr) at the factory line >>(for production devices) >>Examples: >>$ fastboot oem board:write_serialno ABCDEF >>$ fastboot oem board:write_macaddr AA:BB:CC:DD:EE >> >> 2. Access secure storage (via an Trusted Application) > > Agree, you are completely right. > >> >> But both examples could also be in a fastboot flash format: >> $ fastboot flash serialno ABCDEF > > But this case requires to 'serialno' partition definition in schema? > I didn't fully understand you. I meant more in a "conceptual way". (from a end user perspective) "fastboot flash" is generic command that's just supposed to write data somewhere. The back-end (partitioning etc) depends on the storage the device uses so that's a "implementation detail". In any case, I don't have a proper alternative to what you are proposing so as send in [1], I'm okay picking this up after some minor review comments are addressed. [1] https://lore.kernel.org/all/87jzlcfang@baylibre.com/ > >> >> One concern I have is that U-Boot forks might use this command as >> an excuse to not makes things generic. >> >> I hope that others will chime in on this as well. >> I'd like to discuss this more because once this command is in we cannot >> remove it later. >> >> > --- >> > drivers/fastboot/Kconfig | 7 +++ >> > drivers/fastboot/fb_command.c | 15 +++ >> > include/fastboot.h| 1 + >> > 3 files changed, 23 insertions(+) >> > >> > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig >> > index a4313d60a9..4d94391a76 100644 >> > --- a/drivers/fastboot/Kconfig >> > +++ b/drivers/fastboot/Kconfig >> > @@ -241,6 +241,13 @@ config FASTBOOT_OEM_RUN >> > this feature if you are using verified boot, as it will allow an >> > attacker to bypass any restrictions you have in place. >> > >> > +config FASTBOOT_OEM_BOARD >> > + bool "Enable the 'oem board' command" >> > + help >> > +This extends the fastboot protocol with an "oem board" command. This >> > +command allows running vendor custom code defined in board/ files. >> > +Otherwise, it will do nothing and send fastboot fail. >> >> If we move forward with this, please also document the new command in: >> doc/android/fastboot.rst >> >> > + >> > endif # FASTBOOT >> > >> > endmenu >> > diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c >> > index 5fcadcdf50..2298815770 100644 >> > --- a/drivers/fastboot/fb_command.c >> > +++ b/drivers/fastboot/fb_command.c >> > @@ -40,6 +40,7 @@ static void reboot_recovery(char *, char *); >> > static void oem_format(char *, char *); >> > static void oem_partconf(char *, cha
Re: [RFC PATCH v2 1/2] fastboot: introduce 'oem board' subcommand
Hi Alexey,
On mer., avril 03, 2024 at 08:49, Alexey Romanov
wrote:
> Hello Mattijs,
> is there any feedback?
Sorry for the late reply. I was both swamped with other work and awaiting.
feedback from others.
I don't have strong enough arguments to state that this is not useful to
others, I have re-considered this and I'm willing to pick it up.
Please rebase, as this no longer applies.
Also see some review comments below
>
> On Thu, Feb 15, 2024 at 10:14:13AM +0100, Mattijs Korpershoek wrote:
>> Hi Alexey,
>>
>> Thank you for the patch.
>>
>> On jeu., f'evr. 01, 2024 at 12:20, Alexey Romanov
>> wrote:
>>
>> > Currently, fastboot protocol in U-Boot has no opportunity
>> > to execute vendor custom code with verifed boot. This patch
>> > introduce new fastboot subcommand fastboot oem board:,
>> > which allow to run custom oem_board function.
>> >
>> > Default implementation is __weak. Vendor must redefine it in
>> > board/ folder with his own logic.
>> >
>> > For example, some vendors have their custom nand/emmc partition
>> > flashing or erasing. Here some typical command for such use cases:
>> >
>> > - flashing:
>> >
>> > $ fastboot stage bootloader.img
>> > $ fastboot oem board:write_bootloader
>> >
>> > - erasing:
>> >
>> > $ fastboot oem board:erase_env
>> >
>> > Signed-off-by: Alexey Romanov
>>
>> Sorry for the delay. I needed time to give this some thoughts and I
>> waited for Sean to chime as well on this.
>>
>> I've heard from Neil that this might be related to:
>> https://github.com/superna/pyamlboot/pull/20
>>
>> I think this can be useful. Not necessarily for writing custom
>> partitions, but I see this could be used for other things:
>>
>> 1. Provision SoC-specific fuses (serialno/mac addr) at the factory line
>>(for production devices)
>>Examples:
>>$ fastboot oem board:write_serialno ABCDEF
>>$ fastboot oem board:write_macaddr AA:BB:CC:DD:EE
>>
>> 2. Access secure storage (via an Trusted Application)
>>
>> But both examples could also be in a fastboot flash format:
>> $ fastboot flash serialno ABCDEF
>>
>> One concern I have is that U-Boot forks might use this command as
>> an excuse to not makes things generic.
>>
>> I hope that others will chime in on this as well.
>> I'd like to discuss this more because once this command is in we cannot
>> remove it later.
>>
>> > ---
>> > drivers/fastboot/Kconfig | 7 +++
>> > drivers/fastboot/fb_command.c | 15 +++
>> > include/fastboot.h| 1 +
>> > 3 files changed, 23 insertions(+)
>> >
>> > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
>> > index a4313d60a9..4d94391a76 100644
>> > --- a/drivers/fastboot/Kconfig
>> > +++ b/drivers/fastboot/Kconfig
>> > @@ -241,6 +241,13 @@ config FASTBOOT_OEM_RUN
>> > this feature if you are using verified boot, as it will allow an
>> > attacker to bypass any restrictions you have in place.
>> >
>> > +config FASTBOOT_OEM_BOARD
>> > + bool "Enable the 'oem board' command"
>> > + help
>> > +This extends the fastboot protocol with an "oem board" command. This
>> > +command allows running vendor custom code defined in board/ files.
>> > +Otherwise, it will do nothing and send fastboot fail.
>>
>> If we move forward with this, please also document the new command in:
>> doc/android/fastboot.rst
This still applies, document the command please.
>>
>> > +
>> > endif # FASTBOOT
>> >
>> > endmenu
>> > diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
>> > index 5fcadcdf50..2298815770 100644
>> > --- a/drivers/fastboot/fb_command.c
>> > +++ b/drivers/fastboot/fb_command.c
>> > @@ -40,6 +40,7 @@ static void reboot_recovery(char *, char *);
>> > static void oem_format(char *, char *);
>> > static void oem_partconf(char *, char *);
>> > static void oem_bootbus(char *, char *);
>> > +static void oem_board(char *, char *);
>> > static void run_ucmd(char *, char *);
>> > static void run_acmd(char *, char *);
>> >
>> > @@ -107,6 +108,10 @@ static const struct {
>> >.command = "oem run",
>> >.dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_RUN, (run_ucm
Re: [PATCH 4/4] fastboot: integrate block flashing back-end
Hi Dmitrii,
Thank you for the patch and sorry for the review delay.
On mer., mars 06, 2024 at 18:59, Dmitrii Merkurev wrote:
> 1. Get partition info/size
> 2. Erase partition
> 3. Flash partition
> 4. BCB
>
> Signed-off-by: Dmitrii Merkurev
> Cc: Alex Kiernan
> Cc: Patrick Delaunay
> Cc: Simon Glass
> Cc: Mattijs Korpershoek
> Cc: Ying-Chun Liu (PaulLiu)
> ---
> drivers/fastboot/fb_command.c | 8
> drivers/fastboot/fb_common.c | 15 +++
> drivers/fastboot/fb_getvar.c | 8 +++-
> 3 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
> index f95f4e4ae1..67ebe02efa 100644
> --- a/drivers/fastboot/fb_command.c
> +++ b/drivers/fastboot/fb_command.c
> @@ -9,6 +9,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -331,6 +332,10 @@ void fastboot_data_complete(char *response)
> */
> static void __maybe_unused flash(char *cmd_parameter, char *response)
> {
> + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK))
> + fastboot_block_flash_write(cmd_parameter, fastboot_buf_addr,
> +image_size, response);
> +
> if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
> fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr,
>image_size, response);
> @@ -351,6 +356,9 @@ static void __maybe_unused flash(char *cmd_parameter,
> char *response)
> */
> static void __maybe_unused erase(char *cmd_parameter, char *response)
> {
> + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK))
> + fastboot_block_erase(cmd_parameter, response);
> +
> if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
> fastboot_mmc_erase(cmd_parameter, response);
>
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index 3576b06772..7602653c0b 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -97,16 +97,23 @@ int __weak fastboot_set_reboot_flag(enum
> fastboot_reboot_reason reason)
> [FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
> [FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
> };
> - const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> -CONFIG_FASTBOOT_FLASH_MMC_DEV,
> -1);
>
> - if (!IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
> + int device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,
> + CONFIG_FASTBOOT_FLASH_BLOCK_DEVICE_ID,
> -1);
> + if (device == -1) {
> + device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> + CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
> + }
> + char *bcb_iface = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,
Can this be made const?
With the above addressed:
Reviewed-by: Mattijs Korpershoek
> +
> CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME, "mmc");
> +
> + if (device == -1)
> return -EINVAL;
>
> if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
> return -EINVAL;
>
> - ret = bcb_find_partition_and_load("mmc", mmc_dev, "misc");
> + ret = bcb_find_partition_and_load(bcb_iface, device, "misc");
> if (ret)
> goto out;
>
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index f65519c57b..71507009ab 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -8,6 +8,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -114,7 +115,12 @@ static int getvar_get_part_info(const char *part_name,
> char *response,
> struct disk_partition disk_part;
> struct part_info *part_info;
>
> - if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) {
> + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) {
> + r = fastboot_block_get_part_info(part_name, _desc,
> _part,
> + response);
> + if (r >= 0 && size)
> + *size = disk_part.size * disk_part.blksz;
> + } else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) {
> r = fastboot_mmc_get_part_info(part_name, _desc, _part,
> response);
> if (r >= 0 && size)
> --
> 2.44.0.278.ge034bb2e1d-goog
Re: [PATCH 3/4] fastboot: blk: introduce fastboot block flashing support
Hi Dmitrii,
Thank you for the patch and sorry for the review delay.
On mer., mars 06, 2024 at 18:59, Dmitrii Merkurev wrote:
> Reuse common logic between existing mmc and new introduced
> block implementation
>
> Signed-off-by: Dmitrii Merkurev
> Cc: Alex Kiernan
> Cc: Patrick Delaunay
> Cc: Simon Glass
> Cc: Mattijs Korpershoek
> Cc: Ying-Chun Liu (PaulLiu)
This change (when applied along with 1/4 and 2/4) introduces a build for
sandbox:
$ make sandbox_defconfig
$ make
[...]
drivers/fastboot/fb_mmc.c: In function 'fastboot_mmc_erase':
drivers/fastboot/fb_mmc.c:596:16: warning: implicit declaration of function
'fb_block_write'; did you mean 'blk_write'? [-Wimplicit-function-declaration]
596 | blks = fb_block_write(dev_desc, blks_start, blks_size, NULL);
|^~
|blk_write
[...]
LTO u-boot
/usr/bin/ld: /tmp/cczd5cS1.ltrans18.ltrans.o: in function `erase.lto_priv.0':
/mnt/work/upstream/u-boot/drivers/fastboot/fb_mmc.c:596: undefined reference to
`fb_block_write'
collect2: error: ld returned 1 exit status
make: *** [Makefile:1798: u-boot] Error 1
> ---
> drivers/fastboot/Makefile | 4 +-
> drivers/fastboot/fb_block.c | 200
> drivers/fastboot/fb_mmc.c | 120 ++
> include/fb_block.h | 70 +
> 4 files changed, 281 insertions(+), 113 deletions(-)
> create mode 100644 drivers/fastboot/fb_block.c
> create mode 100644 include/fb_block.h
>
> diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
> index 048af5aa82..91e98763e8 100644
> --- a/drivers/fastboot/Makefile
> +++ b/drivers/fastboot/Makefile
> @@ -3,5 +3,7 @@
> obj-y += fb_common.o
> obj-y += fb_getvar.o
> obj-y += fb_command.o
> -obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o
> +obj-$(CONFIG_FASTBOOT_FLASH_BLOCK) += fb_block.o
> +# MMC reuses block implementation
> +obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_block.o fb_mmc.o
> obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o
> diff --git a/drivers/fastboot/fb_block.c b/drivers/fastboot/fb_block.c
> new file mode 100644
> index 00..908decd544
> --- /dev/null
> +++ b/drivers/fastboot/fb_block.c
> @@ -0,0 +1,200 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Copyright (C) 2024 The Android Open Source Project
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/**
> + * FASTBOOT_MAX_BLOCKS_ERASE - maximum blocks to erase per derase call
> + *
> + * in the ERASE case we can have much larger buffer size since
> + * we're not transferring an actual buffer
> + */
> +#define FASTBOOT_MAX_BLOCKS_ERASE 1048576
> +/**
> + * FASTBOOT_MAX_BLOCKS_WRITE - maximum blocks to write per dwrite call
> + */
> +#define FASTBOOT_MAX_BLOCKS_WRITE 65536
> +
> +struct fb_block_sparse {
> + struct blk_desc *dev_desc;
> +};
> +
> +static lbaint_t fb_block_write(struct blk_desc *block_dev, lbaint_t start,
> +lbaint_t blkcnt, const void *buffer)
> +{
> + lbaint_t blk = start;
> + lbaint_t blks_written = 0;
> + lbaint_t cur_blkcnt = 0;
> + lbaint_t blks = 0;
> + int step = buffer ? FASTBOOT_MAX_BLOCKS_WRITE :
> FASTBOOT_MAX_BLOCKS_ERASE;
> + int i;
> +
> + for (i = 0; i < blkcnt; i += step) {
> + cur_blkcnt = min((int)blkcnt - i, step);
> + if (buffer) {
> + if (fastboot_progress_callback)
> + fastboot_progress_callback("writing");
> + blks_written = blk_dwrite(block_dev, blk, cur_blkcnt,
> + buffer + (i *
> block_dev->blksz));
> + } else {
> + if (fastboot_progress_callback)
> + fastboot_progress_callback("erasing");
> + blks_written = blk_derase(block_dev, blk, cur_blkcnt);
> + }
> + blk += blks_written;
> + blks += blks_written;
> + }
> + return blks;
> +}
> +
> +static lbaint_t fb_block_sparse_write(struct sparse_storage *info,
> + lbaint_t blk, lbaint_t blkcnt,
> + const void *buffer)
> +{
> + struct fb_block_sparse *sparse = info->priv;
> + struct blk_desc *dev_desc = sparse->dev_desc;
> +
> + return fb_block_write(dev_desc, blk, blkcnt, buffer);
> +}
> +
> +static lbaint_t fb_block_sparse_reserve(struct sparse_storage *info,
> + lbaint_t blk, lbaint_t blkcnt)
> +{
> +
Re: [PATCH 2/4] fastboot: blk: add block device flashing configuration
Hi Dmitrii, Thank you for the patch and sorry for the review delay. On mer., mars 06, 2024 at 18:59, Dmitrii Merkurev wrote: > Signed-off-by: Dmitrii Merkurev > Cc: Patrick Delaunay > Cc: Simon Glass > Cc: Mattijs Korpershoek > Cc: Ying-Chun Liu (PaulLiu) > --- > drivers/fastboot/Kconfig | 20 ++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig > index 5e5855a76c..460c5e98d7 100644 > --- a/drivers/fastboot/Kconfig > +++ b/drivers/fastboot/Kconfig > @@ -87,7 +87,7 @@ config FASTBOOT_USB_DEV > config FASTBOOT_FLASH > bool "Enable FASTBOOT FLASH command" > default y if ARCH_SUNXI || ARCH_ROCKCHIP > - depends on MMC || (MTD_RAW_NAND && CMD_MTDPARTS) > + depends on MMC || (MTD_RAW_NAND && CMD_MTDPARTS) || BLK > select IMAGE_SPARSE > help > The fastboot protocol includes a "flash" command for writing > @@ -109,12 +109,16 @@ choice > > config FASTBOOT_FLASH_MMC > bool "FASTBOOT on MMC" > - depends on MMC > + depends on MMC && BLK > > config FASTBOOT_FLASH_NAND > bool "FASTBOOT on NAND" > depends on MTD_RAW_NAND && CMD_MTDPARTS > > +config FASTBOOT_FLASH_BLOCK > + bool "FASTBOOT on block device" > + depends on BLK > + If we just apply this patch, then this KConfig option is unused. Can we please squash this into patch 3/4? > endchoice > > config FASTBOOT_FLASH_MMC_DEV > @@ -189,6 +193,18 @@ config FASTBOOT_MMC_USER_NAME > defined here. > The default target name for erasing EMMC_USER is "mmc0". > > +config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME > + string "Define FASTBOOT block interface name" > + depends on FASTBOOT_FLASH_BLOCK > + help > + "Fastboot block interface name (mmc, virtio, etc)" There is a finite list of supported options for this. Can we please document all of them so that users know what is valid and what not? If so, we can drop the "etc" part in the help string. > + > +config FASTBOOT_FLASH_BLOCK_DEVICE_ID > + int "Define FASTBOOT block device id" > + depends on FASTBOOT_FLASH_BLOCK > + help > + "Fastboot block device id" When FASTBOOT_FLASH_BLOCK=="mmc", how is FASTBOOT_FLASH_BLOCK_DEVICE_ID different from FASTBOOT_FLASH_MMC_DEV? If they are similar, are we sure this symbol really needed? > + > config FASTBOOT_GPT_NAME > string "Target name for updating GPT" > depends on FASTBOOT_FLASH_MMC && EFI_PARTITION > -- > 2.44.0.278.ge034bb2e1d-goog
Re: [PATCH 1/4] virtio: blk: introduce virtio-block erase support
Hi Dmitrii,
Thank you for the patch.
I'm not the virtio maintainer in U-Boot:
/mnt/work/upstream/u-boot/ $ ./scripts/get_maintainer.pl drivers/virtio/
Bin Meng (maintainer:VirtIO)
[...]
I've added Bin to the Cc list, please include him if you re-submit.
On mer., mars 06, 2024 at 18:59, Dmitrii Merkurev wrote:
> Co-developed-by: Cody Schuffelen
> Signed-off-by: Cody Schuffelen
> Signed-off-by: Dmitrii Merkurev
> Cc: Tuomas Tynkkynen
> Cc: Simon Glass
> Cc: Mattijs Korpershoek
> Cc: Ying-Chun Liu (PaulLiu)
I'm not super familiar with virtio, so I've not reviewed this.
However, I tested the sandbox unit tests:
uboot@db35472a:/mnt/work/upstream/u-boot$ ./u-boot -T -c "ut dm virtio*"
Bloblist at b000 not found (err=-2)
sandbox_serial serial: pinctrl_select_state_full:
uclass_get_device_by_phandle_id: err=-19
U-Boot 2024.04-00137-g075feb52299 (Apr 05 2024 - 07:12:35 +)
Reset Status: WARM Reset Status: COLD
Model: sandbox
DRAM: 256 MiB
[nvmxip-qspi1@0800]: the block device blk#2 ready for use
[nvmxip-qspi2@0820]: the block device blk#2 ready for use
Core: 286 devices, 101 uclasses, devicetree: board
WDT: Not starting wdt-gpio-toggle
WDT: Not starting wdt-gpio-level
WDT: Not starting wdt@0
NAND: 4100 MiB
MMC: mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD)
Loading Environment from nowhere... OK
In:serial,cros-ec-keyb,usbkbd
Out: serial,vidconsole
Err: serial,vidconsole
Model: sandbox
Net: eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth6: eth@10004000,
eth8: phy-test-eth, eth4: dsa-test-eth, eth2: lan0, eth7: lan1
Test: dm_test_virtio_all_ops: virtio_device.c
Test: dm_test_virtio_all_ops: virtio_device.c (flat tree)
Test: dm_test_virtio_base: virtio_device.c
Test: dm_test_virtio_base: virtio_device.c (flat tree)
Test: dm_test_virtio_missing_ops: virtio.c
Test: dm_test_virtio_missing_ops: virtio.c (flat tree)
Test: dm_test_virtio_remove: virtio_device.c
Test: dm_test_virtio_remove: virtio_device.c (flat tree)
Test: dm_test_virtio_ring: virtio_device.c
Test: dm_test_virtio_ring: virtio_device.c (flat tree)
Test: dm_test_virtio_rng_check_len: virtio_rng.c
Test: dm_test_virtio_rng_check_len: virtio_rng.c (flat tree)
Failures: 0
Tested-by: Mattijs Korpershoek # sandbox
> ---
> drivers/virtio/virtio_blk.c | 91 +++--
> drivers/virtio/virtio_blk.h | 47 +++
> 2 files changed, 124 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/virtio/virtio_blk.c b/drivers/virtio/virtio_blk.c
> index 9581058286..225b65c4d1 100644
> --- a/drivers/virtio/virtio_blk.c
> +++ b/drivers/virtio/virtio_blk.c
> @@ -19,30 +19,82 @@ struct virtio_blk_priv {
> struct virtqueue *vq;
> };
>
> +static const u32 feature[] = {
> + VIRTIO_BLK_F_WRITE_ZEROES
> +};
> +
> +static void virtio_blk_init_header_sg(struct udevice *dev, u64 sector, u32
> type,
> + struct virtio_blk_outhdr *out_hdr, struct
> virtio_sg *sg)
> +{
> + const bool sector_is_needed = type == VIRTIO_BLK_T_IN ||
> + type == VIRTIO_BLK_T_OUT;
> +
> + out_hdr->type = cpu_to_virtio32(dev, type);
> + out_hdr->sector = cpu_to_virtio64(dev, sector_is_needed ? sector : 0);
> +
> + sg->addr = out_hdr;
> + sg->length = sizeof(*out_hdr);
> +}
> +
> +static void virtio_blk_init_write_zeroes_sg(struct udevice *dev, u64 sector,
> lbaint_t blkcnt,
> + struct
> virtio_blk_discard_write_zeroes *wz,
> + struct virtio_sg *sg)
> +{
> + wz->sector = cpu_to_virtio64(dev, sector);
> + wz->num_sectors = cpu_to_virtio32(dev, blkcnt);
> + wz->flags = cpu_to_virtio32(dev, 0);
> +
> + sg->addr = wz;
> + sg->length = sizeof(*wz);
> +}
> +
> +static void virtio_blk_init_status_sg(u8 *status, struct virtio_sg *sg)
> +{
> + sg->addr = status;
> + sg->length = sizeof(*status);
> +}
> +
> +static void virtio_blk_init_data_sg(void *buffer, lbaint_t blkcnt, struct
> virtio_sg *sg)
> +{
> + sg->addr = buffer;
> + sg->length = blkcnt * 512;
> +}
> +
> static ulong virtio_blk_do_req(struct udevice *dev, u64 sector,
> lbaint_t blkcnt, void *buffer, u32 type)
> {
> struct virtio_blk_priv *priv = dev_get_priv(dev);
> + struct virtio_blk_outhdr out_hdr;
> + struct virtio_blk_discard_write_zeroes wz_hdr;
> unsigned int num_out = 0, num_in = 0;
> + struct virtio_sg hdr_sg, wz_sg, data_sg, status_sg;
> struct virtio_sg *sgs[3];
> u8 status;
> int ret;
>
> - struct virtio_blk_outhdr out_hdr = {
> -
[PATCH] doc: build: fix gen_compile_commands section level
The 2 subsections of this page ("Compatible IDEs" and "Usage") are using
the same header level as the title (with "===").
Because of this, they always appear in the HTML rendered ToC.
Drop the subsections one level lower by replacing "=" by "-".
This fixes the HTML rendering.
Signed-off-by: Mattijs Korpershoek
---
doc/build/gen_compile_commands.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/build/gen_compile_commands.rst
b/doc/build/gen_compile_commands.rst
index 50305cec4a83..d503764f9e3f 100644
--- a/doc/build/gen_compile_commands.rst
+++ b/doc/build/gen_compile_commands.rst
@@ -42,7 +42,7 @@ needed for it to be usable by the LSP, unless you set a name
for the database
other than it's default one (compile_commands.json).
Compatible IDEs
-===
+---
Several popular integrated development environments (IDEs) support the use
of JSON compilation databases for C/C++ development, making it easier to
@@ -73,7 +73,7 @@ compile_commands.json in the root of the repository should
suffice to enable
code navigation.
Usage
-=
+-
For further details on the script's options, please refer to its help message,
as in the example below.
---
base-commit: cdfcc37428e06f4730ab9a17cc084eeb7676ea1a
change-id: 20240405-doc-fix-headings-6109365355a3
Best regards,
--
Mattijs Korpershoek
[PATCH] doc: build: update docker image to latest
commit cacc0b2678c0 ("CI: Move to latest container image") updated the
docker container image.
Reflect the change in the documentation.
Signed-off-by: Mattijs Korpershoek
---
Note: maybe we can consider doing the doc change each time we update
.gitlab-ci.yml ?
---
doc/build/docker.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/build/docker.rst b/doc/build/docker.rst
index 953d1b28a063..45659b3b89dc 100644
--- a/doc/build/docker.rst
+++ b/doc/build/docker.rst
@@ -11,4 +11,4 @@ Or to use an existing container
.. code-block:: bash
-sudo docker pull trini/u-boot-gitlab-ci-runner:bionic-20200807-02Sep2020
+sudo docker pull trini/u-boot-gitlab-ci-runner:jammy-20240227-14Mar2024
---
base-commit: cdfcc37428e06f4730ab9a17cc084eeb7676ea1a
change-id: 20240405-doc-container-img-904d56d5efcb
Best regards,
--
Mattijs Korpershoek
Re: [PATCH v1] MAINTAINERS: add entries for tee-related orphaned files
Hi Igor, Thank you for the patch. On jeu., avril 04, 2024 at 12:52, Igor Opaniuk wrote: > Add orphaned TEE/OP-TEE-related files to TEE subsystem entry in > MAINTAINERS. This includes: > - optee_rpmb cmd and test for it > - Misc. OP-TEE tests > - OP-TEE SCMI agent implementation > - Documentation, including device tree bindings > > Signed-off-by: Igor Opaniuk Reviewed-by: Mattijs Korpershoek > --- > > MAINTAINERS | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0462ade4ac6..0f7325d6045 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1642,9 +1642,18 @@ M: Jens Wiklander > M: Ilias Apalodimas > T: git https://source.denx.de/u-boot/custodians/u-boot-tpm.git > S: Maintained > +F: cmd/optee* > +F: doc/README.tee > +F: doc/device-tree-bindings/firmware/linaro,optee-tz.txt > +F: drivers/firmware/scmi/optee_agent.c > F: drivers/tee/ > +F: include/sandboxtee.h > F: include/tee.h > F: include/tee/ > +F: include/test/optee.h > +F: test/dm/tee.c > +F: test/optee/ > +F: test/py/tests/test_optee_rpmb.py > > TEE-lib > M: Bryan O'Donoghue > -- > 2.34.1
[GIT PULL] Please pull u-boot-dfu-20240402
Hi Tom, Here are some fixes for master including: - Fix #if logic in android_ab command - Fix ANDROID_AB_BACKUP_OFFSET in android_ab The CI job is at https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/20142 Thanks, Mattijs The following changes since commit 425dc46a2c8ebe6d1a5a7d2d7ac5506aa7783409: Merge tag 'u-boot-imx-master-20240330' of https://gitlab.denx.de/u-boot/custodians/u-boot-imx (2024-03-30 14:07:37 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-dfu.git tags/u-boot-dfu-20240402 for you to fetch changes up to 7a0cfb87672f727b089844dc635bef01d512816f: android_ab: Fix ANDROID_AB_BACKUP_OFFSET (2024-04-02 09:09:58 +0200) u-boot-dfu-20240402 - Fix #if logic in android_ab command - Fix ANDROID_AB_BACKUP_OFFSET in android_ab Colin McAllister (2): android_ab: Add missing semicolon android_ab: Fix ANDROID_AB_BACKUP_OFFSET boot/android_ab.c | 97 ++- 1 file changed, 45 insertions(+), 52 deletions(-)
[GIT PULL] Please pull u-boot-dfu-next-20240402
Hi Tom, Here are some developments for next including: - Implement Qualcomm wrapper for dwc3 - Multiple sector size support for UMS - CDC ACM gadget initialization fix - Refactor board code from dwc3 to prepare better interrupt support - Bugfix for for qcom-smmu when compiling with -DDEBUG The CI job is at https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/20143 Thanks, Mattijs Note: sorry for the noise, I forgot to include u-boot list in the first email. The following changes since commit e50cb36cb5e772d5bbd30d070faedf7323406364: Merge branch '2024-03-19-assorted-updates' into next (2024-03-20 08:39:05 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-dfu.git tags/u-boot-dfu-next-20240402 for you to fetch changes up to 12ac51cdb788b9f8e50cbc4fa3681102882ade33: usb: udc: dwc3: Fold dwc3_uboot_handle_interrupt into dm_usb_gadget_handle_interrupts (2024-04-02 09:08:07 +0200) u-boot-dfu-next-20240402 - Implement Qualcomm wrapper for dwc3 - Multiple sector size support for UMS - CDC ACM gadget initialization fix - Refactor board code from dwc3 to prepare better interrupt support - Bugfix for for qcom-smmu when compiling with -DDEBUG Caleb Connolly (4): usb: dwc3-generic: implement Qualcomm wrapper usb: gadget: CDC ACM: call usb_gadget_initialize usb: gadget: UMS: support multiple sector sizes iommu: qcom-smmu: fix debugging Marek Vasut (2): usb: udc: dwc3: Fold board dm_usb_gadget_handle_interrupts() into DWC3 gadget usb: udc: dwc3: Fold dwc3_uboot_handle_interrupt into dm_usb_gadget_handle_interrupts board/purism/librem5/spl.c | 6 --- board/samsung/common/exynos5-dt.c | 6 --- board/st/stih410-b2260/board.c | 6 --- board/ti/am43xx/board.c | 11 cmd/usb_mass_storage.c | 4 -- drivers/iommu/qcom-hyp-smmu.c | 2 +- drivers/usb/dwc3/core.c | 26 ++--- drivers/usb/dwc3/dwc3-generic.c | 81 +++- drivers/usb/dwc3/dwc3-omap.c| 4 +- drivers/usb/gadget/f_acm.c | 9 drivers/usb/gadget/f_mass_storage.c | 102 drivers/usb/gadget/storage_common.c | 12 +++-- include/dwc3-omap-uboot.h | 1 - include/dwc3-uboot.h| 2 +- include/usb_mass_storage.h | 1 - 15 files changed, 178 insertions(+), 95 deletions(-)
Re: [PATCH 1/2] usb: udc: dwc3: Fold board dm_usb_gadget_handle_interrupts() into DWC3 gadget
Hi, On Sun, 17 Mar 2024 05:42:52 +0100, Marek Vasut wrote: > The dm_usb_gadget_handle_interrupts() has no place in board code. Move > this into DWC3 driver. The OMAP implementation is special, add new weak > dwc3_uboot_interrupt_status() function to decide whether DWC3 interrupt > handling should be called, and override it in OMAP DWC3 code, to repair > the special OMAP interrupt handling code until OMAP gets switched over > to DM UDC proper. > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next) [1/2] usb: udc: dwc3: Fold board dm_usb_gadget_handle_interrupts() into DWC3 gadget https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/4d158980897085a5b0255ab910208d8afc8522dc [2/2] usb: udc: dwc3: Fold dwc3_uboot_handle_interrupt into dm_usb_gadget_handle_interrupts https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/12ac51cdb788b9f8e50cbc4fa3681102882ade33 -- Mattijs
Re: [PATCH v5 16/16] usb: gadget: UMS: fix 64-bit division on ARM32
Hi Caleb,
Thank you for the patch.
On jeu., mars 28, 2024 at 17:59, Caleb Connolly
wrote:
> The patch introducing support for dynamic sector sizes changed the types
> used in some divisions, resulting in the compiler attempting to use
> libgcc helpers (__aeabi_ldivmod). Replace these divisions with calls to
> lldiv() to handle this correctly.
>
> Fixes: 74e56e0c5065 ("usb: gadget: UMS: support multiple sector sizes")
> Signed-off-by: Caleb Connolly
I believe I applied this already via [1]
I plan to send your work to Tom this week.
Sorry for the delay!
[1] https://lore.kernel.org/r/87a5mqei67@baylibre.com
> ---
> Cc: Mattijs Korpershoek
> ---
> drivers/usb/gadget/f_mass_storage.c | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_mass_storage.c
> b/drivers/usb/gadget/f_mass_storage.c
> index d880928044f4..ef90c7ec7fb5 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -239,8 +239,9 @@
> /* #define VERBOSE_DEBUG */
> /* #define DUMP_MSGS */
>
> #include
> +#include
> #include
> #include
> #include
> #include
> @@ -768,10 +769,10 @@ static int do_read(struct fsg_common *common)
> }
>
> /* Perform the read */
> rc = ums[common->lun].read_sector([common->lun],
> - file_offset / curlun->blksize,
> - amount / curlun->blksize,
> + lldiv(file_offset, curlun->blksize),
> + lldiv(amount, curlun->blksize),
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
>
> @@ -942,10 +943,10 @@ static int do_write(struct fsg_common *common)
> amount = bh->outreq->actual;
>
> /* Perform the write */
> rc = ums[common->lun].write_sector([common->lun],
> -file_offset / curlun->blksize,
> -amount / curlun->blksize,
> +lldiv(file_offset,
> curlun->blksize),
> +lldiv(amount, curlun->blksize),
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
> nwritten = rc * curlun->blksize;
> @@ -1058,10 +1059,10 @@ static int do_verify(struct fsg_common *common)
> }
>
> /* Perform the read */
> rc = ums[common->lun].read_sector([common->lun],
> - file_offset / curlun->blksize,
> - amount / curlun->blksize,
> + lldiv(file_offset, curlun->blksize),
> + lldiv(amount, curlun->blksize),
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
> nread = rc * curlun->blksize;
>
> --
> 2.44.0
Re: [PATCH v2 0/5] arm: meson: Switch to using upstream DT for GXL, GXM, AXG, G12A, G12B & SM1 SoCs
Hi Neil, Thank you for the series. On ven., mars 29, 2024 at 18:51, "Neil Armstrong" wrote: > Enable OF_UPSTREAM to use upstream DT and add amlogic/ prefix to the > DEFAULT_DEVICE_TREE. And thereby directly build DTB from dts/upstream/src/ > including *-u-boot.dtsi files from arch/$(ARCH)/dts/ directory and > drop redundant files from arch/arm/dts directory. Only *-u-boot.dtsi files > kept in arch/arm/dts directory for these boards. > > Keep A1 DTs locally since the architecture is still young. > > CI built & tested at: > https://gitlab.com/amlogic-foss/amlogic-u-boot-autotest/-/pipelines/1233461384 > > Signed-off-by: Neil Armstrong > --- > Changes in v2: > - add change to remove prefix in fdtfile > - collected review tags > - Link to v1: > https://lore.kernel.org/r/20240319-u-boot-of-upstream-v1-0-f82bc9fe6...@linaro.org Boot tested AOSP on VIM3. Tested-by: Mattijs Korpershoek # khadas-vim3_android > > --- > Neil Armstrong (5): > configs: meson64: remove amlogic prefix in fdtfile when > CONFIG_OF_UPSTREAM is selected > dts: meson: Switch GXL, GXM & AXG to using upstream DT > dts: meson: Drop redundant GXL, GXM & AXG devicetree files > dts: meson-g12a: Switch to using upstream DT > dts: meson: Drop redundant G12A, G12B & SM1 devicetree files > > arch/arm/dts/Makefile | 36 +- > arch/arm/dts/meson-axg-jethome-jethub-j100.dts | 361 --- > arch/arm/dts/meson-axg-s400.dts| 602 - > arch/arm/dts/meson-axg.dtsi| 1957 --- > arch/arm/dts/meson-g12-common.dtsi | 2493 > > arch/arm/dts/meson-g12.dtsi| 385 --- > arch/arm/dts/meson-g12a-radxa-zero.dts | 405 > arch/arm/dts/meson-g12a-sei510.dts | 566 - > arch/arm/dts/meson-g12a-u200.dts | 308 --- > arch/arm/dts/meson-g12a.dtsi | 140 -- > arch/arm/dts/meson-g12b-a311d-bananapi-m2s.dts | 33 - > arch/arm/dts/meson-g12b-a311d-khadas-vim3.dts | 41 - > arch/arm/dts/meson-g12b-a311d.dtsi | 149 -- > arch/arm/dts/meson-g12b-bananapi-cm4-cm4io.dts | 165 -- > arch/arm/dts/meson-g12b-bananapi-cm4.dtsi | 388 --- > arch/arm/dts/meson-g12b-bananapi.dtsi | 521 > arch/arm/dts/meson-g12b-gsking-x.dts | 133 -- > arch/arm/dts/meson-g12b-gtking-pro.dts | 142 -- > arch/arm/dts/meson-g12b-gtking.dts | 163 -- > arch/arm/dts/meson-g12b-khadas-vim3.dtsi | 107 - > arch/arm/dts/meson-g12b-odroid-go-ultra.dts| 722 -- > arch/arm/dts/meson-g12b-odroid-n2-plus.dts | 31 - > arch/arm/dts/meson-g12b-odroid-n2.dts | 15 - > arch/arm/dts/meson-g12b-odroid-n2.dtsi | 303 --- > arch/arm/dts/meson-g12b-odroid-n2l.dts | 125 - > arch/arm/dts/meson-g12b-odroid.dtsi| 445 > arch/arm/dts/meson-g12b-radxa-zero2.dts| 489 > arch/arm/dts/meson-g12b-s922x-bananapi-m2s.dts | 14 - > arch/arm/dts/meson-g12b-s922x.dtsi | 139 -- > arch/arm/dts/meson-g12b-w400.dtsi | 425 > arch/arm/dts/meson-g12b.dtsi | 146 -- > arch/arm/dts/meson-gx-libretech-pc.dtsi| 447 > arch/arm/dts/meson-gx-mali450.dtsi | 61 - > arch/arm/dts/meson-gx-p23x-q20x.dtsi | 324 --- > arch/arm/dts/meson-gx.dtsi | 675 -- > arch/arm/dts/meson-gxl-mali.dtsi | 17 - > arch/arm/dts/meson-gxl-s805x-libretech-ac.dts | 319 --- > arch/arm/dts/meson-gxl-s805x.dtsi | 23 - > arch/arm/dts/meson-gxl-s905d-libretech-pc.dts | 16 - > arch/arm/dts/meson-gxl-s905d.dtsi | 12 - > .../arm/dts/meson-gxl-s905w-jethome-jethub-j80.dts | 247 -- > arch/arm/dts/meson-gxl-s905x-khadas-vim.dts| 237 -- > arch/arm/dts/meson-gxl-s905x-libretech-cc-v2.dts | 313 --- > arch/arm/dts/meson-gxl-s905x-libretech-cc.dts | 356 --- > arch/arm/dts/meson-gxl-s905x-p212.dts | 134 -- > arch/arm/dts/meson-gxl-s905x-p212.dtsi | 213 -- > arch/arm/dts/meson-gxl-s905x.dtsi | 18 - > arch/arm/dts/meson-gxl.dtsi| 940 > arch/arm/dts/meson-gxm-gt1-ultimate.dts| 91 - > arch/arm/dts/meson-gxm-khadas-vim2.dts | 424 > arch/arm/dts/meson-gxm-s912-libretech-pc.dts | 62 - > arch/arm/dts/meson-gxm-wetek-core2.dts | 85 - > arch/arm/dts/meson-gxm
Re: [PATCH 0/4] arm: meson: Switch to using upstream DT for GXL, GXM, AXG, G12A, G12B & SM1 SoCs
Hi, On jeu., mars 28, 2024 at 09:21, Neil Armstrong wrote: > Hi Mattijs, > > On 19/03/2024 15:42, Neil Armstrong wrote: >> Enable OF_UPSTREAM to use upstream DT and add amlogic/ prefix to the >> DEFAULT_DEVICE_TREE. And thereby directly build DTB from dts/upstream/src/ >> including *-u-boot.dtsi files from arch/$(ARCH)/dts/ directory and >> drop redundant files from arch/arm/dts directory. Only *-u-boot.dtsi files >> kept in arch/arm/dts directory for these boards. >> >> Keep A1 DTs locally since the architecture is still young. >> >> CI built & tested at: >> https://gitlab.com/amlogic-foss/amlogic-u-boot-autotest/-/pipelines/1219273552 > > Can you check it doesn't break android boot ? Sorry for the delay. I will test this early next week (Tuesday) Thank you ! > > Neil > >> >> Signed-off-by: Neil Armstrong >> --- >> Neil Armstrong (4): >>dts: meson: Switch GXL, GXM & AXG to using upstream DT >>dts: meson: Drop redundant GXL, GXM & AXG devicetree files >>dts: meson-g12a: Switch to using upstream DT >>dts: meson: Drop redundant G12A, G12B & SM1 devicetree files >> >> arch/arm/dts/Makefile | 36 +- >> arch/arm/dts/meson-axg-jethome-jethub-j100.dts | 361 --- >> arch/arm/dts/meson-axg-s400.dts| 602 - >> arch/arm/dts/meson-axg.dtsi| 1957 --- >> arch/arm/dts/meson-g12-common.dtsi | 2493 >> >> arch/arm/dts/meson-g12.dtsi| 385 --- >> arch/arm/dts/meson-g12a-radxa-zero.dts | 405 >> arch/arm/dts/meson-g12a-sei510.dts | 566 - >> arch/arm/dts/meson-g12a-u200.dts | 308 --- >> arch/arm/dts/meson-g12a.dtsi | 140 -- >> arch/arm/dts/meson-g12b-a311d-bananapi-m2s.dts | 33 - >> arch/arm/dts/meson-g12b-a311d-khadas-vim3.dts | 41 - >> arch/arm/dts/meson-g12b-a311d.dtsi | 149 -- >> arch/arm/dts/meson-g12b-bananapi-cm4-cm4io.dts | 165 -- >> arch/arm/dts/meson-g12b-bananapi-cm4.dtsi | 388 --- >> arch/arm/dts/meson-g12b-bananapi.dtsi | 521 >> arch/arm/dts/meson-g12b-gsking-x.dts | 133 -- >> arch/arm/dts/meson-g12b-gtking-pro.dts | 142 -- >> arch/arm/dts/meson-g12b-gtking.dts | 163 -- >> arch/arm/dts/meson-g12b-khadas-vim3.dtsi | 107 - >> arch/arm/dts/meson-g12b-odroid-go-ultra.dts| 722 -- >> arch/arm/dts/meson-g12b-odroid-n2-plus.dts | 31 - >> arch/arm/dts/meson-g12b-odroid-n2.dts | 15 - >> arch/arm/dts/meson-g12b-odroid-n2.dtsi | 303 --- >> arch/arm/dts/meson-g12b-odroid-n2l.dts | 125 - >> arch/arm/dts/meson-g12b-odroid.dtsi| 445 >> arch/arm/dts/meson-g12b-radxa-zero2.dts| 489 >> arch/arm/dts/meson-g12b-s922x-bananapi-m2s.dts | 14 - >> arch/arm/dts/meson-g12b-s922x.dtsi | 139 -- >> arch/arm/dts/meson-g12b-w400.dtsi | 425 >> arch/arm/dts/meson-g12b.dtsi | 146 -- >> arch/arm/dts/meson-gx-libretech-pc.dtsi| 447 >> arch/arm/dts/meson-gx-mali450.dtsi | 61 - >> arch/arm/dts/meson-gx-p23x-q20x.dtsi | 324 --- >> arch/arm/dts/meson-gx.dtsi | 675 -- >> arch/arm/dts/meson-gxl-mali.dtsi | 17 - >> arch/arm/dts/meson-gxl-s805x-libretech-ac.dts | 319 --- >> arch/arm/dts/meson-gxl-s805x.dtsi | 23 - >> arch/arm/dts/meson-gxl-s905d-libretech-pc.dts | 16 - >> arch/arm/dts/meson-gxl-s905d.dtsi | 12 - >> .../arm/dts/meson-gxl-s905w-jethome-jethub-j80.dts | 247 -- >> arch/arm/dts/meson-gxl-s905x-khadas-vim.dts| 237 -- >> arch/arm/dts/meson-gxl-s905x-libretech-cc-v2.dts | 313 --- >> arch/arm/dts/meson-gxl-s905x-libretech-cc.dts | 356 --- >> arch/arm/dts/meson-gxl-s905x-p212.dts | 134 -- >> arch/arm/dts/meson-gxl-s905x-p212.dtsi | 213 -- >> arch/arm/dts/meson-gxl-s905x.dtsi | 18 - >> arch/arm/dts/meson-gxl.dtsi| 940 >> arch/arm/dts/meson-gxm-gt1-ultimate.dts| 91 - >> arch/arm/dts/meson-gxm-khadas-vim2.dts | 424 >> arch/arm/dts/meson-gxm-s912-libretech-pc.dts | 62 - >> arch/arm/dts/meson-gxm-wetek-core2.dts | 85 - >> arch/arm/dts/meson-gxm.dtsi| 216 -- >> arch/arm/dts/meson-khadas-vim3.dtsi| 534 - >> arch/arm/dts/meson-sm1-bananapi-m2-pro.dts | 97 - >> arch/arm/dts/meson-sm1-bananapi-m5.dts | 221 -- >> arch/arm/dts/meson-sm1-bananapi.dtsi | 435 >> arch/arm/dts/meson-sm1-khadas-vim3l.dts
Re: [PATCH 1/2] usb: udc: dwc3: Fold board dm_usb_gadget_handle_interrupts() into DWC3 gadget
On jeu., mars 21, 2024 at 20:38, Marek Vasut wrote: > On 3/21/24 11:01 AM, Mattijs Korpershoek wrote: >> Hi Marek, >> >> Thank you for the patch. >> >> On dim., mars 17, 2024 at 05:42, Marek Vasut >> wrote: >> >>> The dm_usb_gadget_handle_interrupts() has no place in board code. Move >>> this into DWC3 driver. The OMAP implementation is special, add new weak >>> dwc3_uboot_interrupt_status() function to decide whether DWC3 interrupt >>> handling should be called, and override it in OMAP DWC3 code, to repair >>> the special OMAP interrupt handling code until OMAP gets switched over >>> to DM UDC proper. >>> >>> Signed-off-by: Marek Vasut >> >> Reviewed-by: Mattijs Korpershoek >> >> Tested that I could scan for usb mass storage and that I can use >> fastboot: >> >> => fastboot usb 0 >> crq->brequest:0x0 >> => usb start >> starting USB... >> Bus usb@ff50: Register 3000140 NbrPorts 3 >> Starting the controller >> USB XHCI 1.10 >> scanning bus usb@ff50 for devices... 3 USB Device(s) found >> scanning usb for storage devices... 1 Storage Device(s) found >> >> => usb storage >>Device 0: Vendor: SanDisk Rev: 1.00 Prod: Ultra >> Type: Removable Hard Disk >> Capacity: 29328.0 MB = 28.6 GB (60063744 x 512) >> >> Tested-by: Mattijs Korpershoek # vim3 > > Thank you! > > Please feel free to pick this via DFU tree in a week or two, so others > can comment on this and test this too. Will do!
Re: [PATCH] usb: gadget: UMS: fix 64-bit division on ARM32
Hi Caleb,
On jeu., mars 21, 2024 at 17:01, Caleb Connolly
wrote:
> On 21/03/2024 16:48, Mattijs Korpershoek wrote:
>> Hi Caleb,
>>
>> Thank you for the patch.
>>
>> On jeu., mars 21, 2024 at 15:28, Caleb Connolly
>> wrote:
>>
>>> The patch introducing support for dynamic sector sizes changed the types
>>> used in some divisions, resulting in the compiler attempting to use
>>> libgcc helpers (__aeabi_ldivmod). Replace these divisions with calls to
>>> lldiv() to handle this correctly.
>>>
>>> Fixes: 74e56e0c5065 ("usb: gadget: UMS: support multiple sector sizes")
>>> Signed-off-by: Caleb Connolly
>>
>> Reviewed-by: Mattijs Korpershoek
>>
>> Would it be okay if I squashed this into
>> commit 74e56e0c5065 ("usb: gadget: UMS: support multiple sector sizes")
>> ?
>>
>> I'd like to not (intentionally) send any patches that don't compile to Tom.
>
> For sure, feel free to do that.
Done, here:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/304fa0aa445384e5e681a54abf413850591cec10
>>
>>
>>> ---
>>> Hi Mattijs,
>>>
>>> Sorry for not catching that sooner! I compile tested this locally and a
>>
>> No worries, thanks a lot for the quick fix.
>>
>>> CI pipeline is running here:
>>> https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/pipelines/20029
>>
>> I can see from the above pipeline that some other failures are ongoing,
>> but they are not related to lldiv().
>>
>> arm-linux-gnueabi-ld.bfd: common/dfu.o: in function `run_usb_dnl_gadget':
>> common/dfu.c:82:(.text.run_usb_dnl_gadget+0xd8): undefined reference to
>> `dm_usb_gadget_handle_interrupts'
>> arm-linux-gnueabi-ld.bfd: common/dfu.c:108:(.text.run_usb_dnl_gadget+0x11c):
>> undefined reference to `dm_usb_gadget_handle_interrupts'
>> make[1]: *** [Makefile:1793: u-boot] Error
>
> Ah! I guess this is caused by the "select DM_USB_GADGET" patch...
>
> You can revert it for now and I'll just select DM_USB_GADGET for qcom.
Will do, I'll drop it from the branch.
>
> It seems like there's a bunch going on around here with the cleanup by
> Marek. I can revisit this once that's settled
Yeah, we are working on reworking the interrupt handling a bit since it
has quite some limitations.
>>
>> See:
>> https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/jobs/802759#L470
>>
>> I suspect that this is related to:
>> https://lore.kernel.org/r/all/20240225152715.1821613-1-jo...@kwiboo.se/
>
> Is this fixed by the ("usb: udc: dwc3: Fold board
> dm_usb_gadget_handle_interrupts() into DWC3") series?
Unfortunately, no:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/jobs/803228#L578
>>
>>
>>>
>>> Regards,
>>> Caleb
>>> ---
>>> drivers/usb/gadget/f_mass_storage.c | 13 +++--
>>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/usb/gadget/f_mass_storage.c
>>> b/drivers/usb/gadget/f_mass_storage.c
>>> index d880928044f4..ef90c7ec7fb5 100644
>>> --- a/drivers/usb/gadget/f_mass_storage.c
>>> +++ b/drivers/usb/gadget/f_mass_storage.c
>>> @@ -239,8 +239,9 @@
>>> /* #define VERBOSE_DEBUG */
>>> /* #define DUMP_MSGS */
>>>
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> #include
>>> @@ -768,10 +769,10 @@ static int do_read(struct fsg_common *common)
>>> }
>>>
>>> /* Perform the read */
>>> rc = ums[common->lun].read_sector([common->lun],
>>> - file_offset / curlun->blksize,
>>> - amount / curlun->blksize,
>>> + lldiv(file_offset, curlun->blksize),
>>> + lldiv(amount, curlun->blksize),
>>> (char __user *)bh->buf);
>>> if (!rc)
>>> return -EIO;
>>>
>>> @@ -942,10 +943,10 @@ static int do_write(struct fsg_common *common)
>>> amount = bh->outreq->actual;
>>>
>>> /* Perform the write */
>>> rc = ums[common->lun].write_sector([common->lun],
>>> - file_offset / curlun->blksize,
>>> -
Re: [PATCH v4 0/2] Fix Android A/B backup
Hi, On Tue, 12 Mar 2024 07:57:27 -0500, Colin McAllister wrote: > - Addresses compiler error due to missing semicolon > - Removes use of preprocessor macros with ANDROID_AB_BACKUP_OFFSET > > Bug was found by noticing a semicolon was missing and not causing a > compiler error when CONFIG_ANDROID_AB_BACKUP_OFFSET was set. I submitted > a patch to fix the semicolon before fixing the #if's. Testing the latter > patch without the former with ANDORID_AB_BACKUP_OFFSET set will cause a > compiler error. > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu) [1/2] android_ab: Add missing semicolon https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/c044f89bb0c2683bcb14006b697ee6db2240f00d [2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/026e0cd31278bcafcfb35d21478a94fc5072f78c -- Mattijs
Re: [PATCH] usb: gadget: UMS: fix 64-bit division on ARM32
Hi Caleb,
Thank you for the patch.
On jeu., mars 21, 2024 at 15:28, Caleb Connolly
wrote:
> The patch introducing support for dynamic sector sizes changed the types
> used in some divisions, resulting in the compiler attempting to use
> libgcc helpers (__aeabi_ldivmod). Replace these divisions with calls to
> lldiv() to handle this correctly.
>
> Fixes: 74e56e0c5065 ("usb: gadget: UMS: support multiple sector sizes")
> Signed-off-by: Caleb Connolly
Reviewed-by: Mattijs Korpershoek
Would it be okay if I squashed this into
commit 74e56e0c5065 ("usb: gadget: UMS: support multiple sector sizes")
?
I'd like to not (intentionally) send any patches that don't compile to Tom.
> ---
> Hi Mattijs,
>
> Sorry for not catching that sooner! I compile tested this locally and a
No worries, thanks a lot for the quick fix.
> CI pipeline is running here:
> https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/pipelines/20029
I can see from the above pipeline that some other failures are ongoing,
but they are not related to lldiv().
arm-linux-gnueabi-ld.bfd: common/dfu.o: in function `run_usb_dnl_gadget':
common/dfu.c:82:(.text.run_usb_dnl_gadget+0xd8): undefined reference to
`dm_usb_gadget_handle_interrupts'
arm-linux-gnueabi-ld.bfd: common/dfu.c:108:(.text.run_usb_dnl_gadget+0x11c):
undefined reference to `dm_usb_gadget_handle_interrupts'
make[1]: *** [Makefile:1793: u-boot] Error
See:
https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/jobs/802759#L470
I suspect that this is related to:
https://lore.kernel.org/r/all/20240225152715.1821613-1-jo...@kwiboo.se/
>
> Regards,
> Caleb
> ---
> drivers/usb/gadget/f_mass_storage.c | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_mass_storage.c
> b/drivers/usb/gadget/f_mass_storage.c
> index d880928044f4..ef90c7ec7fb5 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -239,8 +239,9 @@
> /* #define VERBOSE_DEBUG */
> /* #define DUMP_MSGS */
>
> #include
> +#include
> #include
> #include
> #include
> #include
> @@ -768,10 +769,10 @@ static int do_read(struct fsg_common *common)
> }
>
> /* Perform the read */
> rc = ums[common->lun].read_sector([common->lun],
> - file_offset / curlun->blksize,
> - amount / curlun->blksize,
> + lldiv(file_offset, curlun->blksize),
> + lldiv(amount, curlun->blksize),
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
>
> @@ -942,10 +943,10 @@ static int do_write(struct fsg_common *common)
> amount = bh->outreq->actual;
>
> /* Perform the write */
> rc = ums[common->lun].write_sector([common->lun],
> -file_offset / curlun->blksize,
> -amount / curlun->blksize,
> +lldiv(file_offset,
> curlun->blksize),
> +lldiv(amount, curlun->blksize),
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
> nwritten = rc * curlun->blksize;
> @@ -1058,10 +1059,10 @@ static int do_verify(struct fsg_common *common)
> }
>
> /* Perform the read */
> rc = ums[common->lun].read_sector([common->lun],
> - file_offset / curlun->blksize,
> - amount / curlun->blksize,
> + lldiv(file_offset, curlun->blksize),
> + lldiv(amount, curlun->blksize),
> (char __user *)bh->buf);
> if (!rc)
> return -EIO;
> nread = rc * curlun->blksize;
> --
> 2.44.0
Re: [PATCH v4 0/5] Qualcomm DWC3 USB support
Hi Caleb, It seems CI got broken with this series. Baseline CI (passed): https://source.denx.de/u-boot/u-boot/-/pipelines/20006 With series applied (failed): https://source.denx.de/u-boot/custodians/u-boot-dfu/-/jobs/802278 It seems the change on UMS introduced some linking errors on arm32: f_mass_storage.c:946:(.text.do_write.lto_priv.0+0x1de): undefined reference to `__aeabi_ldivmod' Could you have a look, please? Thank you. Mattijs On jeu., mars 21, 2024 at 10:23, Mattijs Korpershoek wrote: > Hi, > > On Wed, 20 Mar 2024 14:30:46 +, Caleb Connolly wrote: >> This series enables support for Qualcomm platforms in the DWC3 driver, >> adds support for arbitrary sector sizes to the USB mass storage gadget, >> and fixes an issue with the CDC ACM driver where it wouldn't initialise >> the USB device. >> >> Additionally, it fixes a syntax bug in the Qualcomm SMMU driver, and >> makes USB_DWC3_GADGET select DM_USB_GADGET to fix compilation with >> gadget mode. >> >> [...] > > Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu > (u-boot-dfu-next) > > [1/5] usb: dwc3-generic: implement Qualcomm wrapper > > https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/850b307789dca8dfff6d7cad57ba2ffbfbb5f6ed > [2/5] usb: dwc3: select DM_USB_GADGET > > https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/97d629372c8c58da96f46aa6d2bec09f120cc62d > [3/5] usb: gadget: CDC ACM: call usb_gadget_initialize > > https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/86327b4e3201a55aefee2f1bfbbea9a7896461f0 > [4/5] usb: gadget: UMS: support multiple sector sizes > > https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/7e71eeb39f4c5280b1ce677f4a14063feaedff95 > [5/5] iommu: qcom-smmu: fix debugging > > https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/c9959ce7b54145a83d3797bcfb3c3713cb4a97ea > > -- > Mattijs
Re: [PATCH 1/5] usb: dwc3-generic: implement Qualcomm wrapper
On jeu., mars 21, 2024 at 11:34, Caleb Connolly
wrote:
> Hi,
>
> On 21/03/2024 09:25, Mattijs Korpershoek wrote:
>> Hi Caleb, Marek,
>>
>> On jeu., mars 21, 2024 at 06:34, Marek Vasut wrote:
>>
>>> On 3/13/24 7:22 PM, Caleb Connolly wrote:
>>>
>>> [...]
>>>
>>>>>> +static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset,
>>>>>> u32 val)
>>>>>> +{
>>>>>> + u32 reg;
>>>>>> +
>>>>>> + reg = readl(base + offset);
>>>>>> + reg |= val;
>>>>>> + writel(reg, base + offset);
>>>>>
>>>>> Use setbits_le32() .
>>>>>
>>>>>> + /* ensure that above write is through */
>>>>>> + readl(base + offset);
>>>>>
>>>>> Is this needed ?
>>>>
>>>> I honestly don't know, this is copied from the Linux driver and it seems
>>>> to be very defensively written. I doubt it's strictly necessary.
>>>
>>> Does git log indicate anything ?
>
> Nope :/ it's there from when the driver was first added.
>>>
>>> I suspect this is some sort of barrier .
>>>
>>> [...]
>>>
>>>>>> +/* For controllers running without superspeed PHYs */
>>>>>> +static void dwc3_qcom_select_utmi_clk(void __iomem *qscratch_base)
>>>>>> +{
>>>>>> + /* Configure dwc3 to use UTMI clock as PIPE clock not present */
>>>>>> + dwc3_qcom_setbits(qscratch_base, QSCRATCH_GENERAL_CFG,
>>>>>> + PIPE_UTMI_CLK_DIS);
>>>>>> +
>>>>>> + udelay(500);
>>>>>
>>>>> Isn't there some possibility to poll for completion instead of fixed
>>>>> delay ? If so, use wait_for_bit or some such .
>>>>
>>>> Not that I'm aware of, no. I think this hardware just has a blanket
>>>> "writes take X bus cycles to complete" rule or something. It's totally
>>>> possible that this code was originally written this way to work around
>>>> some issues on an FPGA prototype or something. Everything seems to still
>>>> work if I remove the delays so I'll drop them...
>>>
>>> Could you possibly ask someone ?
>
> Yeah I'll ask around, I'm not confident I'll find an answer though.
>>>
>>> [...]
>>>
>>>>>> static int dwc3_rk_glue_get_ctrl_dev(struct udevice *dev, ofnode
>>>>>> *node)
>>>>>> {
>>>>>> *node = dev_ofnode(dev);
>>>>>> @@ -506,6 +599,10 @@ static int dwc3_glue_reset_init(struct udevice *dev,
>>>>>> else if (ret)
>>>>>> return ret;
>>>>>> + if (device_is_compatible(dev, "qcom,dwc3")) {
>>>>>> + reset_assert_bulk(>resets);
>>>>>> + udelay(500);
>>>>>
>>>>> Why this delay here ?
>>>>
>>>> According to the docs, the reset should be asserted for at least 6 sleep
>>>> clock cycles, that's ~200us on sdm845, but it can vary by platform.
>>>
>>> A comment in the code would be nice.
>>>
>>> Sorry for the abysmal delay in replies.
>>>
>>> btw. the new version of this series is still OK to go in, unless you
>>> want to fill in the comments. They can also go in in separate follow up
>>> patch.
>>
>> I'm interested by the answers above as well. As I took in the series [1] (to
>> avoid delaying it too much), please consider a follow up patch to add a
>> comment.
>
> The v4 you picked up has a comment explaining this.
Right, sorry I missed that. Thanks for pointing it out to me!
>>
>> [1]
>> https://lore.kernel.org/r/all/171101299073.1017001.16411913317437946645.b4...@baylibre.com/
>>
>
> --
> // Caleb (they/them)
Re: [PATCH v3] test: dm: add button_cmd test
Hi Caleb,
Thank you for the patch.
On mar., mars 19, 2024 at 13:24, Caleb Connolly
wrote:
> Add a test for the button_cmd feature. This validates that commands can
> be mapped to two buttons, that the correct command runs based on which
> button is pressed, that only 1 command is run, and that no command runs
> if button_cmd_0_name is wrong or unset.
>
> Additionally, fix a potential uninitialised variable use caught by these
> tests, the btn variable in get_button_cmd() is assumed to be null if
> button_get_by_label() fails, but it's actually used uninitialised in
> that case.
>
> CONFIG_BUTTON is now enabled automatically and was removed when running
> save_defconfig.
>
> Fixes: e761035b6423 ("boot: add support for button commands")
> Signed-off-by: Caleb Connolly
Reviewed-by: Mattijs Korpershoek
> ---
> Pipeline:
> https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/pipelines/19995
> Changes in v3:
> - Enable CONFIG_BUTTON_CMD for sandbox_flattree as well.
> - Link to v2:
> https://lore.kernel.org/u-boot/20240305145111.1391645-1-caleb.conno...@linaro.org
>
> Changes in v2:
> - Explicitly assign btn as NULL in get_button_cmd(). This fixes a
> bug where if the undefined variable is non-zero the
> button_get_by_label() check would fail and result in invalid memory
> being accessed.
> - Enable CONFIG_BUTTON_CMD for sandbox64 as well as sandbox.
> - Link to v1:
> https://lore.kernel.org/u-boot/20240214170357.4091708-1-caleb.conno...@linaro.org/
> ---
> common/button_cmd.c| 2 +-
> configs/sandbox64_defconfig| 1 +
> configs/sandbox_defconfig | 1 +
> configs/sandbox_flattree_defconfig | 1 +
> test/dm/button.c | 96 ++
> 5 files changed, 100 insertions(+), 1 deletion(-)
>
> diff --git a/common/button_cmd.c b/common/button_cmd.c
> index b6a8434d6f29..8642c26735cc 100644
> --- a/common/button_cmd.c
> +++ b/common/button_cmd.c
> @@ -32,9 +32,9 @@ struct button_cmd {
> */
> static int get_button_cmd(int n, struct button_cmd *cmd)
> {
> const char *cmd_str;
> - struct udevice *btn;
> + struct udevice *btn = NULL;
> char buf[24];
>
> snprintf(buf, sizeof(buf), "button_cmd_%d_name", n);
> cmd->btn_name = env_get(buf);
> diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> index 3be9a00a8575..a62faf772482 100644
> --- a/configs/sandbox64_defconfig
> +++ b/configs/sandbox64_defconfig
> @@ -10,8 +10,9 @@ CONFIG_PCI=y
> CONFIG_SANDBOX64=y
> CONFIG_DEBUG_UART=y
> CONFIG_SYS_MEMTEST_START=0x0010
> CONFIG_SYS_MEMTEST_END=0x00101000
> +CONFIG_BUTTON_CMD=y
> CONFIG_FIT=y
> CONFIG_FIT_SIGNATURE=y
> CONFIG_FIT_VERBOSE=y
> CONFIG_LEGACY_IMAGE_FORMAT=y
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 4ad10363e91b..93b52f2de5cf 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -9,8 +9,9 @@ CONFIG_SYS_LOAD_ADDR=0x0
> CONFIG_PCI=y
> CONFIG_DEBUG_UART=y
> CONFIG_SYS_MEMTEST_START=0x0010
> CONFIG_SYS_MEMTEST_END=0x00101000
> +CONFIG_BUTTON_CMD=y
> CONFIG_FIT=y
> CONFIG_FIT_RSASSA_PSS=y
> CONFIG_FIT_CIPHER=y
> CONFIG_FIT_VERBOSE=y
> diff --git a/configs/sandbox_flattree_defconfig
> b/configs/sandbox_flattree_defconfig
> index 039018627527..6bf8874e722e 100644
> --- a/configs/sandbox_flattree_defconfig
> +++ b/configs/sandbox_flattree_defconfig
> @@ -7,8 +7,9 @@ CONFIG_SYS_LOAD_ADDR=0x0
> CONFIG_PCI=y
> CONFIG_DEBUG_UART=y
> CONFIG_SYS_MEMTEST_START=0x0010
> CONFIG_SYS_MEMTEST_END=0x00101000
> +CONFIG_BUTTON_CMD=y
> CONFIG_FIT=y
> CONFIG_FIT_SIGNATURE=y
> CONFIG_FIT_VERBOSE=y
> CONFIG_LEGACY_IMAGE_FORMAT=y
> diff --git a/test/dm/button.c b/test/dm/button.c
> index 3318668df25a..830d96fbef34 100644
> --- a/test/dm/button.c
> +++ b/test/dm/button.c
> @@ -130,4 +130,100 @@ static int dm_test_button_keys_adc(struct
> unit_test_state *uts)
>
> return 0;
> }
> DM_TEST(dm_test_button_keys_adc, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
> +
> +/* Test of the button uclass using the button_gpio driver */
> +static int dm_test_button_cmd(struct unit_test_state *uts)
> +{
> + struct udevice *btn1_dev, *btn2_dev, *gpio;
> + const char *envstr;
> +
> +#define BTN1_GPIO 3
> +#define BTN2_GPIO 4
> +#define BTN1_PASS_VAR "test_button_cmds_0"
> +#define BTN2_PASS_VAR "test_button_cmds_1"
> +
> + /*
> + * Buttons 1 and 2 are connected to gpio_a gpios 3 and 4 respectively.
> + * set the GPIOs to known values and then check that the appropri
Re: [PATCH 2/2] usb: udc: dwc3: Fold dwc3_uboot_handle_interrupt into dm_usb_gadget_handle_interrupts
Hi Marek,
Thank you for the patch.
On dim., mars 17, 2024 at 05:42, Marek Vasut
wrote:
> The only call site of dwc3_uboot_handle_interrupt() is the
> dm_usb_gadget_handle_interrupts(), fold the former into the
> later. This makes dwc3_uboot_handle_interrupt() unavailable
> to be called from board code as well.
>
> Signed-off-by: Marek Vasut
> ---
> Cc: Angus Ainslie
> Cc: Caleb Connolly
> Cc: Eugen Hristev
> Cc: Igor Prusov
> Cc: Mattijs Korpershoek
> Cc: Nishanth Menon
> Cc: Patrice Chotard
> Cc: Simon Glass
> Cc: Stefan Bosch
> Cc: Svyatoslav Ryhel
> Cc: Tom Rini
> Cc: ker...@puri.sm
Reviewed-by: Mattijs Korpershoek
Tested that I could scan for usb mass storage and that I can use
fastboot:
=> fastboot usb 0
crq->brequest:0x0
=> usb start
starting USB...
Bus usb@ff50: Register 3000140 NbrPorts 3
Starting the controller
USB XHCI 1.10
scanning bus usb@ff50 for devices... 3 USB Device(s) found
scanning usb for storage devices... 1 Storage Device(s) found
=> usb storage
Device 0: Vendor: SanDisk Rev: 1.00 Prod: Ultra
Type: Removable Hard Disk
Capacity: 29328.0 MB = 28.6 GB (60063744 x 512)
Tested-by: Mattijs Korpershoek # vim3
> ---
> drivers/usb/dwc3/core.c | 27 +++
> include/dwc3-uboot.h| 1 -
> 2 files changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 09737be9a9c..96e850b7170 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -988,18 +988,27 @@ MODULE_AUTHOR("Felipe Balbi ");
> MODULE_LICENSE("GPL v2");
> MODULE_DESCRIPTION("DesignWare USB3 DRD Controller Driver");
>
> +#if !CONFIG_IS_ENABLED(DM_USB_GADGET)
> +__weak int dwc3_uboot_interrupt_status(struct udevice *dev)
> +{
> + return 1;
> +}
> +
> /**
> - * dwc3_uboot_handle_interrupt - handle dwc3 core interrupt
> + * dm_usb_gadget_handle_interrupts - handle dwc3 core interrupt
> * @dev: device of this controller
> *
> * Invokes dwc3 gadget interrupts.
> *
> * Generally called from board file.
> */
> -void dwc3_uboot_handle_interrupt(struct udevice *dev)
> +int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> {
> struct dwc3 *dwc = NULL;
>
> + if (!dwc3_uboot_interrupt_status(dev))
> + return 0;
> +
> list_for_each_entry(dwc, _list, list) {
> if (dwc->dev != dev)
> continue;
> @@ -1007,20 +1016,6 @@ void dwc3_uboot_handle_interrupt(struct udevice *dev)
> dwc3_gadget_uboot_handle_interrupt(dwc);
> break;
> }
> -}
> -
> -#if !CONFIG_IS_ENABLED(DM_USB_GADGET)
> -__weak int dwc3_uboot_interrupt_status(struct udevice *dev)
> -{
> - return 1;
> -}
> -
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - if (!dwc3_uboot_interrupt_status(dev))
> - return 0;
> -
> - dwc3_uboot_handle_interrupt(dev);
>
> return 0;
> }
> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h
> index 5f13f5bcf40..3689d60ae7f 100644
> --- a/include/dwc3-uboot.h
> +++ b/include/dwc3-uboot.h
> @@ -45,7 +45,6 @@ struct dwc3_device {
> int dwc3_uboot_init(struct dwc3_device *dev);
> void dwc3_uboot_exit(int index);
> int dwc3_uboot_interrupt_status(struct udevice *dev);
> -void dwc3_uboot_handle_interrupt(struct udevice *dev);
>
> struct phy;
> #if CONFIG_IS_ENABLED(PHY) && CONFIG_IS_ENABLED(DM_USB)
> --
> 2.43.0
Re: [PATCH 1/2] usb: udc: dwc3: Fold board dm_usb_gadget_handle_interrupts() into DWC3 gadget
Hi Marek,
Thank you for the patch.
On dim., mars 17, 2024 at 05:42, Marek Vasut
wrote:
> The dm_usb_gadget_handle_interrupts() has no place in board code. Move
> this into DWC3 driver. The OMAP implementation is special, add new weak
> dwc3_uboot_interrupt_status() function to decide whether DWC3 interrupt
> handling should be called, and override it in OMAP DWC3 code, to repair
> the special OMAP interrupt handling code until OMAP gets switched over
> to DM UDC proper.
>
> Signed-off-by: Marek Vasut
Reviewed-by: Mattijs Korpershoek
Tested that I could scan for usb mass storage and that I can use
fastboot:
=> fastboot usb 0
crq->brequest:0x0
=> usb start
starting USB...
Bus usb@ff50: Register 3000140 NbrPorts 3
Starting the controller
USB XHCI 1.10
scanning bus usb@ff50 for devices... 3 USB Device(s) found
scanning usb for storage devices... 1 Storage Device(s) found
=> usb storage
Device 0: Vendor: SanDisk Rev: 1.00 Prod: Ultra
Type: Removable Hard Disk
Capacity: 29328.0 MB = 28.6 GB (60063744 x 512)
Tested-by: Mattijs Korpershoek # vim3
> ---
> Cc: Angus Ainslie
> Cc: Caleb Connolly
> Cc: Eugen Hristev
> Cc: Igor Prusov
> Cc: Mattijs Korpershoek
> Cc: Nishanth Menon
> Cc: Patrice Chotard
> Cc: Simon Glass
> Cc: Stefan Bosch
> Cc: Svyatoslav Ryhel
> Cc: Tom Rini
> Cc: ker...@puri.sm
> ---
> board/purism/librem5/spl.c| 6 --
> board/samsung/common/exynos5-dt.c | 6 --
> board/st/stih410-b2260/board.c| 6 --
> board/ti/am43xx/board.c | 11 ---
> drivers/usb/dwc3/core.c | 25 +
> drivers/usb/dwc3/dwc3-omap.c | 4 ++--
> include/dwc3-omap-uboot.h | 1 -
> include/dwc3-uboot.h | 1 +
> 8 files changed, 24 insertions(+), 36 deletions(-)
>
> diff --git a/board/purism/librem5/spl.c b/board/purism/librem5/spl.c
> index 581f0929662..9aadc553302 100644
> --- a/board/purism/librem5/spl.c
> +++ b/board/purism/librem5/spl.c
> @@ -418,12 +418,6 @@ out:
> return rv;
> }
>
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - dwc3_uboot_handle_interrupt(dev);
> - return 0;
> -}
> -
> static void dwc3_nxp_usb_phy_init(struct dwc3_device *dwc3)
> {
> u32 RegData;
> diff --git a/board/samsung/common/exynos5-dt.c
> b/board/samsung/common/exynos5-dt.c
> index 95cf6d2acc2..b3e87c93751 100644
> --- a/board/samsung/common/exynos5-dt.c
> +++ b/board/samsung/common/exynos5-dt.c
> @@ -122,12 +122,6 @@ static struct dwc3_device dwc3_device_data = {
> .index = 0,
> };
>
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - dwc3_uboot_handle_interrupt(dev);
> - return 0;
> -}
> -
> int board_usb_init(int index, enum usb_init_type init)
> {
> struct exynos_usb3_phy *phy = (struct exynos_usb3_phy *)
> diff --git a/board/st/stih410-b2260/board.c b/board/st/stih410-b2260/board.c
> index e21cbc270e9..82817571ae3 100644
> --- a/board/st/stih410-b2260/board.c
> +++ b/board/st/stih410-b2260/board.c
> @@ -50,12 +50,6 @@ static struct dwc3_device dwc3_device_data = {
> .index = 0,
> };
>
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - dwc3_uboot_handle_interrupt(dev);
> - return 0;
> -}
> -
> int board_usb_init(int index, enum usb_init_type init)
> {
> int node;
> diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c
> index ea0d0b92088..a4679a2e294 100644
> --- a/board/ti/am43xx/board.c
> +++ b/board/ti/am43xx/board.c
> @@ -759,17 +759,6 @@ static struct ti_usb_phy_device usb_phy2_device = {
> .usb2_phy_power = (void *)USB2_PHY2_POWER,
> .index = 1,
> };
> -
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - u32 status;
> -
> - status = dwc3_omap_uboot_interrupt_status(dev);
> - if (status)
> - dwc3_uboot_handle_interrupt(dev);
> -
> - return 0;
> -}
> #endif /* CONFIG_USB_DWC3 */
>
> #if defined(CONFIG_USB_DWC3) || defined(CONFIG_USB_XHCI_OMAP)
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 4b4fcd8a22e..09737be9a9c 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -983,6 +983,11 @@ void dwc3_uboot_exit(int index)
> }
> }
>
> +MODULE_ALIAS("platform:dwc3");
> +MODULE_AUTHOR("Felipe Balbi ");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("DesignWare USB3 DRD Controller Driver");
> +
> /**
> * dwc3_uboot_handle_interrupt - handle dwc3 core interrupt
> * @dev: device of t
Re: [PATCH v4 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET
Hi Colin,
Since both below remarks are minor nitpicks, I can also do them when
applying (to avoid delaying this series too much).
Please le me know what you prefer:
1. You send a v5 at your convience
2. I do the minor fixups and I merge right away.
Again, thank you for doing your first U-Boot contributions!
Mattijs
On mer., mars 13, 2024 at 18:22, Igor Opaniuk wrote:
> Hi Colin,
>
> On Tue, Mar 12, 2024 at 4:19 PM Mattijs Korpershoek <
> mkorpersh...@baylibre.com> wrote:
>
>> Hi Colin,
>>
>> Thank you for the patch.
>>
>> On mar., mars 12, 2024 at 07:57, Colin McAllister
>> wrote:
>>
>> Sam also gave his review here:
>>
>> https://lore.kernel.org/all/CAPLW+4kHmPtfACyND4Vc2p0ZrsyGY=+bRU=fdub4k1ux5p3...@mail.gmail.com/
>>
>> Please include his review tag in the next submission.
>>
>> I will add it at the appropriate place below:
>>
>>
>> > From: Colin McAllister
>> >
>> > Currently, setting CONFIG_AB_BACKUP_OFFSET in a target's defconfig will
>> > not actually enable the #if protected code in android_ab.c. This is
>> > because "CONFIG_" should have been prepended to the config macro, or the
>> > macros defined in kconfig.h could have been used.
>> >
>> > The code included by ANDROID_AB_BACKUP_OFFSET has been refactored to no
>> > longer be conditionally compiled by preprocessor conditionals and
>> > instead use C conditionals. This better aligns with the Linux kernel
>> > style guide.
>> >
>> > Fixes: 3430f24bc6 ("android_ab: Try backup booloader_message")
>> > Signed-off-by: Colin McAllister
>> > Cc: Joshua Watt
>> > Cc: Simon Glass
>> > Signed-off-by: Colin McAllister
>> Reviewed-by: Sam Protsenko
>>
>> > ---
>> > v2:
>> > - Replaced #if conditionals with C if conditionals
>> > - Opted to use CONFIG_ANDROID_AB_BACKUP_OFFSET directly instead of
>> > macros in kconfig.h as CONFIG_ANDROID_AB_BACKUP_OFFSET is not a
>> > boolean or tristate value and doesn't have different values when
>> > building SPL or TPL.
>> > v3:
>> > - Added "Fixes:" tag
>> > v4:
>> > - No changes
>> >
>> > boot/android_ab.c | 97 ++-
>> > 1 file changed, 45 insertions(+), 52 deletions(-)
>> >
>> > diff --git a/boot/android_ab.c b/boot/android_ab.c
>> > index 9a3d15ec60..f547aa64e4 100644
>> > --- a/boot/android_ab.c
>> > +++ b/boot/android_ab.c
>> > @@ -187,13 +187,12 @@ int ab_select_slot(struct blk_desc *dev_desc,
>> struct disk_partition *part_info,
>> > bool dec_tries)
>> > {
>> > struct bootloader_control *abc = NULL;
>> > + struct bootloader_control *backup_abc = NULL;
>> > u32 crc32_le;
>> > int slot, i, ret;
>> > bool store_needed = false;
>> > + bool valid_backup = false;
>> > char slot_suffix[4];
>> > -#if ANDROID_AB_BACKUP_OFFSET
>> > - struct bootloader_control *backup_abc = NULL;
>> > -#endif
>> >
>> > ret = ab_control_create_from_disk(dev_desc, part_info, , 0);
>> > if (ret < 0) {
>> > @@ -205,53 +204,49 @@ int ab_select_slot(struct blk_desc *dev_desc,
>> struct disk_partition *part_info,
>> > return ret;
>> > }
>> >
>> > -#if ANDROID_AB_BACKUP_OFFSET
>> > - ret = ab_control_create_from_disk(dev_desc, part_info, _abc,
>> > - ANDROID_AB_BACKUP_OFFSET);
>> > - if (ret < 0) {
>> > - free(abc);
>> > - return ret;
>> > + if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
>> > + ret = ab_control_create_from_disk(dev_desc, part_info,
>> _abc,
>> > +
>> CONFIG_ANDROID_AB_BACKUP_OFFSET);
>> > + if (ret < 0) {
>> > + free(abc);
>> > + return ret;
>> > + }
>> > }
>> > -#endif
>> >
>> > crc32_le = ab_control_compute_crc(abc);
>> > if (abc->crc32_le != crc32_le) {
>> > log_err("ANDROID: Invalid CRC-32 (expected %.8x, found
>> %.8x),",
>> > crc32_le, abc->crc32_le);
>> > -#if ANDROID_AB_BACKUP_OFFSET
>> > - crc32_le = ab_control_compute_crc(backup_abc);
>>
Re: [PATCH 1/5] usb: dwc3-generic: implement Qualcomm wrapper
Hi Caleb, Marek,
On jeu., mars 21, 2024 at 06:34, Marek Vasut wrote:
> On 3/13/24 7:22 PM, Caleb Connolly wrote:
>
> [...]
>
+static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset,
u32 val)
+{
+ u32 reg;
+
+ reg = readl(base + offset);
+ reg |= val;
+ writel(reg, base + offset);
>>>
>>> Use setbits_le32() .
>>>
+ /* ensure that above write is through */
+ readl(base + offset);
>>>
>>> Is this needed ?
>>
>> I honestly don't know, this is copied from the Linux driver and it seems
>> to be very defensively written. I doubt it's strictly necessary.
>
> Does git log indicate anything ?
>
> I suspect this is some sort of barrier .
>
> [...]
>
+/* For controllers running without superspeed PHYs */
+static void dwc3_qcom_select_utmi_clk(void __iomem *qscratch_base)
+{
+ /* Configure dwc3 to use UTMI clock as PIPE clock not present */
+ dwc3_qcom_setbits(qscratch_base, QSCRATCH_GENERAL_CFG,
+ PIPE_UTMI_CLK_DIS);
+
+ udelay(500);
>>>
>>> Isn't there some possibility to poll for completion instead of fixed
>>> delay ? If so, use wait_for_bit or some such .
>>
>> Not that I'm aware of, no. I think this hardware just has a blanket
>> "writes take X bus cycles to complete" rule or something. It's totally
>> possible that this code was originally written this way to work around
>> some issues on an FPGA prototype or something. Everything seems to still
>> work if I remove the delays so I'll drop them...
>
> Could you possibly ask someone ?
>
> [...]
>
static int dwc3_rk_glue_get_ctrl_dev(struct udevice *dev, ofnode *node)
{
*node = dev_ofnode(dev);
@@ -506,6 +599,10 @@ static int dwc3_glue_reset_init(struct udevice *dev,
else if (ret)
return ret;
+ if (device_is_compatible(dev, "qcom,dwc3")) {
+ reset_assert_bulk(>resets);
+ udelay(500);
>>>
>>> Why this delay here ?
>>
>> According to the docs, the reset should be asserted for at least 6 sleep
>> clock cycles, that's ~200us on sdm845, but it can vary by platform.
>
> A comment in the code would be nice.
>
> Sorry for the abysmal delay in replies.
>
> btw. the new version of this series is still OK to go in, unless you
> want to fill in the comments. They can also go in in separate follow up
> patch.
I'm interested by the answers above as well. As I took in the series [1] (to
avoid delaying it too much), please consider a follow up patch to add a
comment.
[1]
https://lore.kernel.org/r/all/171101299073.1017001.16411913317437946645.b4...@baylibre.com/
Re: [PATCH v4 0/5] Qualcomm DWC3 USB support
Hi, On Wed, 20 Mar 2024 14:30:46 +, Caleb Connolly wrote: > This series enables support for Qualcomm platforms in the DWC3 driver, > adds support for arbitrary sector sizes to the USB mass storage gadget, > and fixes an issue with the CDC ACM driver where it wouldn't initialise > the USB device. > > Additionally, it fixes a syntax bug in the Qualcomm SMMU driver, and > makes USB_DWC3_GADGET select DM_USB_GADGET to fix compilation with > gadget mode. > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next) [1/5] usb: dwc3-generic: implement Qualcomm wrapper https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/850b307789dca8dfff6d7cad57ba2ffbfbb5f6ed [2/5] usb: dwc3: select DM_USB_GADGET https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/97d629372c8c58da96f46aa6d2bec09f120cc62d [3/5] usb: gadget: CDC ACM: call usb_gadget_initialize https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/86327b4e3201a55aefee2f1bfbbea9a7896461f0 [4/5] usb: gadget: UMS: support multiple sector sizes https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/7e71eeb39f4c5280b1ce677f4a14063feaedff95 [5/5] iommu: qcom-smmu: fix debugging https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/c9959ce7b54145a83d3797bcfb3c3713cb4a97ea -- Mattijs
Re: [PATCH v4 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET
ab_control_default(abc);
> if (ret < 0) {
> -#if ANDROID_AB_BACKUP_OFFSET
> - free(backup_abc);
> -#endif
> + if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
> + free(backup_abc);
> free(abc);
> return -ENODATA;
> }
> -#if ANDROID_AB_BACKUP_OFFSET
> - } else {
> - /*
> - * Backup is valid. Copy it to the primary
> - */
> - memcpy(abc, backup_abc, sizeof(*abc));
> }
> -#endif
> store_needed = true;
> }
>
> if (abc->magic != BOOT_CTRL_MAGIC) {
> log_err("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic);
> -#if ANDROID_AB_BACKUP_OFFSET
> - free(backup_abc);
> -#endif
> + if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
> + free(backup_abc);
> free(abc);
> return -ENODATA;
> }
> @@ -259,9 +254,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
> disk_partition *part_info,
> if (abc->version > BOOT_CTRL_VERSION) {
> log_err("ANDROID: Unsupported A/B metadata version: %.8x\n",
> abc->version);
> -#if ANDROID_AB_BACKUP_OFFSET
> - free(backup_abc);
> -#endif
> + if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
> + free(backup_abc);
> free(abc);
> return -ENODATA;
> }
> @@ -338,30 +332,29 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
> disk_partition *part_info,
> abc->crc32_le = ab_control_compute_crc(abc);
> ret = ab_control_store(dev_desc, part_info, abc, 0);
> if (ret < 0) {
> -#if ANDROID_AB_BACKUP_OFFSET
> - free(backup_abc);
> -#endif
> + if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
> + free(backup_abc);
> free(abc);
> return ret;
> }
> }
>
> -#if ANDROID_AB_BACKUP_OFFSET
> - /*
> - * If the backup doesn't match the primary, write the primary
> - * to the backup offset
> - */
> - if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) {
> - ret = ab_control_store(dev_desc, part_info, abc,
> -ANDROID_AB_BACKUP_OFFSET);
> - if (ret < 0) {
> - free(backup_abc);
> - free(abc);
> - return ret;
> + if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
> + /*
> + * If the backup doesn't match the primary, write the primary
> + * to the backup offset
> + */
checkpatch.pl seems to complain about this comment block:
WARNING: Block comments should align the * on each line
#166: FILE: boot/android_ab.c:344:
+ /*
+ * If the backup doesn't match the primary, write the primary
To reproduce, run:
$ ./scripts/checkpatch.pl --strict --u-boot --git HEAD^..HEAD
With the above addressed, please send a v5 including:
Reviewed-by: Mattijs Korpershoek
> + if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) {
> + ret = ab_control_store(dev_desc, part_info, abc,
> +CONFIG_ANDROID_AB_BACKUP_OFFSET);
> + if (ret < 0) {
> + free(backup_abc);
> + free(abc);
> + return ret;
> + }
> }
> + free(backup_abc);
> }
> - free(backup_abc);
> -#endif
>
> free(abc);
>
> --
> 2.34.1
Re: [PATCH v4 1/2] android_ab: Add missing semicolon
Hi Colin,
Thank you for the patch.
On mar., mars 12, 2024 at 07:57, Colin McAllister wrote:
> From: Colin McAllister
>
> Found a missing semicolon in code protected by a #if that will never
> evaluate to true due to a separate issue. Fixing this issue before
> addressing the #if.
>
> Fixes: 3430f24bc6 ("android_ab: Try backup booloader_message")
> Signed-off-by: Colin McAllister
> Cc: Joshua Watt
> Cc: Simon Glass
> Signed-off-by: Colin McAllister
Reviewed-by: Mattijs Korpershoek
Since Sam gave his review in [1]:
https://lore.kernel.org/all/CAPLW+4kHmPtfACyND4Vc2p0ZrsyGY=+bRU=fdub4k1ux5p3...@mail.gmail.com/
I will also add:
Reviewed-by: Sam Protsenko
When applying.
> ---
> v2: No changes
> v3: Added "Fixes:" tag
> v4: No changes
>
> boot/android_ab.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/boot/android_ab.c b/boot/android_ab.c
> index c9df6d2b4b..9a3d15ec60 100644
> --- a/boot/android_ab.c
> +++ b/boot/android_ab.c
> @@ -221,7 +221,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
> disk_partition *part_info,
> #if ANDROID_AB_BACKUP_OFFSET
> crc32_le = ab_control_compute_crc(backup_abc);
> if (backup_abc->crc32_le != crc32_le) {
> - log_err("ANDROID: Invalid backup CRC-32 ")
> + log_err("ANDROID: Invalid backup CRC-32 ");
> log_err("expected %.8x, found %.8x),",
> crc32_le, backup_abc->crc32_le);
> #endif
> --
> 2.34.1
RE: [PATCH v3 0/2] Fix Android A/B backup
Hi Colin, On mar., mars 12, 2024 at 14:04, "McAllister, Colin" wrote: > Hi Mattijs, > > I’ve been using git send-email, but there might be issues with what the > Garmin smtp server is doing to the email, like adding the footer. I sent a v4 > PS in a new thread using my personal email, but that email isn’t subscribed > to this ML so I think the patches are pending approval to be added to the ML. Yep, seems they got approved. I will follow-up on the v4 series. > > Best, > Colin > > From: Mattijs Korpershoek > Sent: Tuesday, March 12, 2024 4:47 AM > To: Sam Protsenko ; McAllister, Colin > > Cc: u-boot@lists.denx.de; jpewhac...@gmail.com; s...@chromium.org; Igor > Opaniuk > Subject: Re: [PATCH v3 0/2] Fix Android A/B backup > > Hi Colin, On ven. , mars 08, 2024 at 15: 59, Sam Protsenko linaro. org> wrote: > On Fri, Mar 8, 2024 at 1: 24 PM McAllister, Colin > > wrote: >> >> > Ah, ok, I see you > > > Hi Colin, > > > > On ven., mars 08, 2024 at 15:59, Sam Protsenko > mailto:semen.protse...@linaro.org>> wrote: > > > >> On Fri, Mar 8, 2024 at 1:24 PM McAllister, Colin > >> mailto:colin.mcallis...@garmin.com>> wrote: > >>> > >>> > Ah, ok, I see you replied to my comment here. > >>> > >>> Yes, sorry. Outlook is terrible to send inline responses too. I figured > >>> just adding responses in the patch contents would be better. Next time I'll >>> submit > >>> my patch with a different email :) > >>> > >>> > So when that config option is not defined at all, the build still > >>> > works, right? > >>> > >>> Yes, the default value for CONFIG_ANDROID_AB_BACKUP_OFFSET is 0x0, which > >>> would evaluate to a false bool value in the if conditions. I did do some > >>> testing with the config value not defined for my board and confirmed > >>> back-up data is not used. > >>> > >> > >> Looks good to me, thanks. > >> > >>> In your other emails you include your reviewed-by tag. For clarity, Am I > >>> supposed to append my patches and upload a new version? This is my > >>> first time contributing to u-boot, so I'm still learning the workflow. I > >>> didn't see anything glancing through the "Sending patches" page in the > >>> U-Boot documentation. > >>> > >> > >> Welcome to the community! And thanks for your patches :) U-Boot > >> workflow is quite similar to Linux kernel one. It's useful to collect > >> all tags when sending out your next version. When the maintainer takes > >> your patch, they usually also apply all R-b tags for the final patch > >> version, so you only have to worry about that when sending out a new > >> version. I know that U-Boot contributors are often using `patman' tool > >> [1] for submitting patches (and corresponding updated versions), and > >> I'm pretty sure it collects all pending tags automatically for you. > >> FWIW, I'm not experienced with `patman', as I'm trying to use somehow > >> unified submitting process for both U-Boot and Linux kernel, and I > >> know that using `patman' is sometimes discouraged in Linux kernel > >> community. > > > > Welcome to the U-Boot community! It takes quite some time to start > > contributing so thank you for the patches. > > > > The changes look fine and the detailed approach on how you tested is > > very much appreciated. > > > > I have a couple of suggestions on the whole series. > > > > 1. The patches don't apply: > > > > $ b4 shazam -s -l > 20240308165937.270499-1-colin.mcallis...@garmin.com<mailto:20240308165937.270499-1-colin.mcallis...@garmin.com> > > > > error: patch failed: boot/android_ab.c:187 > > error: boot/android_ab.c: patch does not apply > > error: Did you hand edit your patch? > > It does not apply to blobs recorded in its index. > > Patch failed at 0002 android_ab: Fix ANDROID_AB_BACKUP_OFFSET > > hint: Use 'git am --show-current-patch=diff' to see the failed patch > > When you have resolved this problem, run "git am --continue". > > If you prefer to skip this patch, run "git am --skip" instead. > > To restore the original branch and stop patching, run "git am --abort". > > > > I suspect your email provider swapped tabs to spaces. It's also possible > > that the Garmin confidentiality footer caused this. > > > > 2. Using the khadas-vim3_android_ab_defconfig, th
Re: [PATCH v2 1/2] android_ab: Add missing semicolon
On jeu., mars 07, 2024 at 16:54, Sam Protsenko
wrote:
> On Thu, Mar 7, 2024 at 4:11 PM Colin McAllister
> wrote:
>>
>> Found a missing semicolon in code protected by a #if that will never
>> evaluate to true due to a separate issue. Fixing this issue before
>> addressing the #if.
>>
>> Signed-off-by: Colin McAllister
>> Cc: Joshua Watt
>> Cc: Simon Glass
>> ---
>> boot/android_ab.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/boot/android_ab.c b/boot/android_ab.c
>> index c9df6d2b4b..9a3d15ec60 100644
>> --- a/boot/android_ab.c
>> +++ b/boot/android_ab.c
>> @@ -221,7 +221,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
>> disk_partition *part_info,
>> #if ANDROID_AB_BACKUP_OFFSET
>> crc32_le = ab_control_compute_crc(backup_abc);
>> if (backup_abc->crc32_le != crc32_le) {
>> - log_err("ANDROID: Invalid backup CRC-32 ")
>> + log_err("ANDROID: Invalid backup CRC-32 ");
>
> Good catch! Wonder why there is also a trailing space in the end of
> the string. Anyways, I think this patch deserves "Fixes:" tag, would
> you mind adding it? Other than that:
The file seems to have weird trailing/leading whitespaces in some of the
error messages.
I think it's fine to keep this as is for now.
We can make another series to fix this after Colin's work gets merged.
>
> Reviewed-by: Sam Protsenko
>
>> log_err("expected %.8x, found %.8x),",
>> crc32_le, backup_abc->crc32_le);
>> #endif
>> --
>> 2.43.2
>>
>>
>>
>>
>> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use
>> of the intended recipient(s) and contain information that may be Garmin
>> confidential and/or Garmin legally privileged. If you have received this
>> email in error, please notify the sender by reply email and delete the
>> message. Any disclosure, copying, distribution or use of this communication
>> (including attachments) by someone other than the intended recipient is
>> prohibited. Thank you.
Re: [PATCH v3 0/2] Fix Android A/B backup
Hi Colin, On ven., mars 08, 2024 at 15:59, Sam Protsenko wrote: > On Fri, Mar 8, 2024 at 1:24 PM McAllister, Colin > wrote: >> >> > Ah, ok, I see you replied to my comment here. >> >> Yes, sorry. Outlook is terrible to send inline responses too. I figured >> just adding responses in the patch contents would be better. Next time I'll >> submit >> my patch with a different email :) >> >> > So when that config option is not defined at all, the build still >> > works, right? >> >> Yes, the default value for CONFIG_ANDROID_AB_BACKUP_OFFSET is 0x0, which >> would evaluate to a false bool value in the if conditions. I did do some >> testing with the config value not defined for my board and confirmed >> back-up data is not used. >> > > Looks good to me, thanks. > >> In your other emails you include your reviewed-by tag. For clarity, Am I >> supposed to append my patches and upload a new version? This is my >> first time contributing to u-boot, so I'm still learning the workflow. I >> didn't see anything glancing through the "Sending patches" page in the >> U-Boot documentation. >> > > Welcome to the community! And thanks for your patches :) U-Boot > workflow is quite similar to Linux kernel one. It's useful to collect > all tags when sending out your next version. When the maintainer takes > your patch, they usually also apply all R-b tags for the final patch > version, so you only have to worry about that when sending out a new > version. I know that U-Boot contributors are often using `patman' tool > [1] for submitting patches (and corresponding updated versions), and > I'm pretty sure it collects all pending tags automatically for you. > FWIW, I'm not experienced with `patman', as I'm trying to use somehow > unified submitting process for both U-Boot and Linux kernel, and I > know that using `patman' is sometimes discouraged in Linux kernel > community. Welcome to the U-Boot community! It takes quite some time to start contributing so thank you for the patches. The changes look fine and the detailed approach on how you tested is very much appreciated. I have a couple of suggestions on the whole series. 1. The patches don't apply: $ b4 shazam -s -l 20240308165937.270499-1-colin.mcallis...@garmin.com error: patch failed: boot/android_ab.c:187 error: boot/android_ab.c: patch does not apply error: Did you hand edit your patch? It does not apply to blobs recorded in its index. Patch failed at 0002 android_ab: Fix ANDROID_AB_BACKUP_OFFSET hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". I suspect your email provider swapped tabs to spaces. It's also possible that the Garmin confidentiality footer caused this. 2. Using the khadas-vim3_android_ab_defconfig, this does not build: boot/android_ab.c: In function 'ab_select_slot': boot/android_ab.c:350:48: error: 'ANDROID_AB_BACKUP_OFFSET' undeclared (first use in this function); did you mean 'CONFIG_ANDROID_AB_BACKUP_OFFSET'? 350 | ANDROID_AB_BACKUP_OFFSET); |^~~~ | CONFIG_ANDROID_AB_BACKUP_OFFSET Both are minor problems. I re-applied the diffs manually to be able to build/boot test this. Since this is your first contribution, I can either: - fix both myself and merge this. - let you spin a v4 to fix the above. Please let me know what you prefer. If you do intend to send a v4, please: - Do it in a new email thread - Make sure you cc me, Igor and Sam - Make sure the patches you send can be applied. http://git-send-email.io/ is a friendly service you can use to test your workflow. On workflow, tooling, I usually suggest the b4 tool: https://people.kernel.org/monsieuricon/sending-a-kernel-patch-with-b4-part-1 Regards, Mattijs > > [1] https://docs.u-boot.org/en/latest/develop/patman.html > > [snip]
Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
Hi Igor, Sam, On mar., févr. 20, 2024 at 20:08, Igor Opaniuk wrote: > Hi Sam, > > On Tue, Feb 20, 2024 at 7:29 PM Sam Protsenko > wrote: >> >> On Mon, Feb 19, 2024 at 4:16 AM Igor Opaniuk wrote: >> > >> > This takes the latest changes from AOSP from [1][2] (as this >> > header was split on two) with minimal changes (this could lead >> > to warnings reported by checkpatch). >> >> Do we want to maybe follow that and also carry two different headers >> in U-Boot? Or it doesn't make much sense? I'm thinking in terms of >> future portability mostly: how easy it's to update this header right >> now, and how easy it's going to be further. I didn't form any opinion >> on that, hence asking. > The problem is licensing. android_bootloader_message.h was > re-licensed by Alex Deymo from Google under BSD-3-Clause, > which is GPLv2 compatible. I'm not sure it's legally correct to pull > boot_control_definition.h from AOSP licensed under Apache as a > separate file. I'd prefer a separate file as well but I'm not familiar enough with licensing to see if that's a problem. I'm ok to keep as is. > >> >> Another thing: are you sure that changing only the header won't break >> anything in U-Boot .c files that use this header? > > I've tested both ab_select and avb verify in QEMU. Or do you mean > something else additionally should be tested? I've boot tested this on VIM3 and tested adb reboot bootloader, adb reboot fastboot (which writes things into the misc partition) > >> >> > >> > Some local changes have been applied: >> >> Is it possible to split this work into two patches: >> 1. Bring the original changes only >> 2. Apply all necessary changes for U-Boot >> >> Or does it break the build, etc? Again, thinking in terms of >> portability easiness, and not sure which approach is better -- just >> asking basically. > Yeah, that's the problem, as splitting this on two commits > will lead to the first one reporting warnings/notes. > >> >> > 1. Enable static_assert() for defined structures to be sure >> > that all of them have correct sizes. >> > 2. Adjuste types in bootloader_control structure with bitfields >> >> Adjuste -> adjust > Will fix, thanks! >> >> > (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields >> >> I wonder if all those extra changes can be upstreamed back to AOSP? >> Ideally we'd want to just copy those headers over from AOSP to U-Boot >> with no changes, would make the porting work easier. What are your >> thoughts on that? > Technically we can, I was planning to do that. I'm not sure this change will be of interest to AOSP, since they moved to the AIDL implementation here: https://android.googlesource.com/platform/hardware/interfaces/+/refs/heads/main/boot/aidl/default/ If you submit it on gerrit, can you please cc me? (using this email address) > >> >> > that cross the width of the type. Changing the type doesn't change >> > the layout though. >> > This addresses this gcc note: >> > In file included from boot/android_ab.c:7: >> > include/android_bootloader_message.h:230:1: note: offset of packed >> > bit-field ‘merge_status’ has changed in GCC 4.4 >> > 230 | } __attribute__((packed)); >> > >> > [1] >> > https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h >> > [2] >> > https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h >> > >> > CC: Alex Deymo >> > CC: Sam Protsenko >> > CC: Eugeniu Rosca >> > CC: Simon Glass >> > Signed-off-by: Igor Opaniuk >> > --- >> > >> > include/android_bootloader_message.h | 104 +++ >> > 1 file changed, 92 insertions(+), 12 deletions(-) >> > >> > diff --git a/include/android_bootloader_message.h >> > b/include/android_bootloader_message.h >> > index 286d7ab0f31..75198fc9dc2 100644 >> > --- a/include/android_bootloader_message.h >> > +++ b/include/android_bootloader_message.h >> > @@ -21,17 +21,22 @@ >> > * stddef.h >> > */ >> > #include >> > +#include >> > #endif >> > >> > // Spaces used by misc partition are as below: >> > // 0 - 2K For bootloader_message >> > // 2K - 16KUsed by Vendor's bootloader (the 2K - 4K range may be >> > optionally used >> > // as bootloader_message_ab struct) >> > -// 16K - 64KUsed by uncrypt and recovery to store wipe_package for >> > A/B devices >> > +// 16K - 32KUsed by uncrypt and recovery to store wipe_package for >> > A/B devices >> > +// 32K - 64KSystem space, used for miscellanious AOSP features. See >> > below. >> > // Note that these offsets are admitted by bootloader,recovery and >> > uncrypt, so they >> > // are not configurable without changing all of them. >> > static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0; >> > +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024; >> > static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 *
Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
Hi Igor,
Thank you for the patch.
On lun., févr. 19, 2024 at 11:15, Igor Opaniuk wrote:
> This takes the latest changes from AOSP from [1][2] (as this
> header was split on two) with minimal changes (this could lead
> to warnings reported by checkpatch).
>
> Some local changes have been applied:
> 1. Enable static_assert() for defined structures to be sure
> that all of them have correct sizes.
> 2. Adjuste types in bootloader_control structure with bitfields
> (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
> that cross the width of the type. Changing the type doesn't change
> the layout though.
> This addresses this gcc note:
> In file included from boot/android_ab.c:7:
> include/android_bootloader_message.h:230:1: note: offset of packed bit-field
> ‘merge_status’ has changed in GCC 4.4
> 230 | } __attribute__((packed));
>
> [1]
> https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
> [2]
> https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
nitpick: Can you please put in the commit message the sha-1 from when this was
synced?
Looking at platform/bootable/recovery, I can see that there are some new
structures introduced (misc_control_message) in
commit 5a4a41ee2e4b ("misctrl: read message, incl 16kb flag")
To do the review, I used:
* //bootable/recovery: a4aec2f2ceac ("Add kcmdline bootloader message")
* //hardware/interfaces: c4b2f5b564e3 ("Merge "Merge Android 14 QPR2 to AOSP
main" into main")
>
> CC: Alex Deymo
> CC: Sam Protsenko
> CC: Eugeniu Rosca
> CC: Simon Glass
> Signed-off-by: Igor Opaniuk
Tested on Khadas VIM3 that I could reboot into fastbootd (recovery)
and into fastboot(u-boot) from Android using:
$ adb reboot fastboot
$ adb reboot bootloader
Reviewed-by: Mattijs Korpershoek
Tested-by: Mattijs Korpershoek # on vim3
> ---
>
> include/android_bootloader_message.h | 104 +++
> 1 file changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/include/android_bootloader_message.h
> b/include/android_bootloader_message.h
> index 286d7ab0f31..75198fc9dc2 100644
> --- a/include/android_bootloader_message.h
> +++ b/include/android_bootloader_message.h
> @@ -21,17 +21,22 @@
> * stddef.h
> */
> #include
> +#include
> #endif
>
> // Spaces used by misc partition are as below:
> // 0 - 2K For bootloader_message
> // 2K - 16KUsed by Vendor's bootloader (the 2K - 4K range may be
> optionally used
> // as bootloader_message_ab struct)
> -// 16K - 64KUsed by uncrypt and recovery to store wipe_package for A/B
> devices
> +// 16K - 32KUsed by uncrypt and recovery to store wipe_package for A/B
> devices
> +// 32K - 64KSystem space, used for miscellanious AOSP features. See
> below.
> // Note that these offsets are admitted by bootloader,recovery and uncrypt,
> so they
> // are not configurable without changing all of them.
> static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
> +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
> static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
> +static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
> +static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
>
> /* Bootloader Message (2-KiB)
> *
> @@ -81,24 +86,67 @@ struct bootloader_message {
> char reserved[1184];
> };
>
> +// Holds Virtual A/B merge status information. Current version is 1. New
> fields
> +// must be added to the end.
> +struct misc_virtual_ab_message {
> + uint8_t version;
> + uint32_t magic;
> + uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
> + uint8_t source_slot; // Slot number when merge_status was written.
> + uint8_t reserved[57];
> +} __attribute__((packed));
> +
> +struct misc_memtag_message {
> + uint8_t version;
> + uint32_t magic; // magic string for treble compat
> + uint32_t memtag_mode;
> + uint8_t reserved[55];
> +} __attribute__((packed));
> +
> +struct misc_kcmdline_message {
> + uint8_t version;
> + uint32_t magic;
> + uint64_t kcmdline_flags;
> + uint8_t reserved[51];
> +} __attribute__((packed));
> +
> +#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
> +#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
> +
> +#define MISC_MEMTAG_MESSAGE_VERSION 1
> +#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
> +#define MISC_MEMTAG_MODE_MEMTAG 0x1
> +#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
> +#define MISC_MEMTAG_MODE_MEMTAG_K
Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled
Hi Jonas, thank you for the patch. On lun., févr. 26, 2024 at 13:36, Jonas Karlman wrote: > On 2024-02-26 11:18, Marek Vasut wrote: >> On 2/26/24 10:50 AM, Jonas Karlman wrote: >>> On 2024-02-26 09:22, Marek Vasut wrote: On 2/26/24 8:54 AM, Jonas Karlman wrote: > On 2024-02-26 02:47, Tom Rini wrote: >> On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote: >>> On 2024-02-25 23:01, Marek Vasut wrote: On 2/25/24 4:27 PM, Jonas Karlman wrote: > Build fail with the following error when DM_USB_GADGET is enabled and > USB_DWC3_GADGET is disabled: > > dwc3/dwc3-generic.o: in function > `dm_usb_gadget_handle_interrupts': > > dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10): >undefined reference to `dwc3_gadget_uboot_handle_interrupt' > > Build also fail with the following error when USB_GADGET_DWC2_OTG + > DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled: > > gadget/dwc2_udc_otg.o: in function > `dm_usb_gadget_handle_interrupts': > gadget/dwc2_udc_otg.c:947: multiple definition of > `dm_usb_gadget_handle_interrupts'; >dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here > > Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET. > > Signed-off-by: Jonas Karlman > --- > drivers/usb/dwc3/dwc3-generic.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-generic.c > b/drivers/usb/dwc3/dwc3-generic.c > index 6fb2de8a5ace..891d01957619 100644 > --- a/drivers/usb/dwc3/dwc3-generic.c > +++ b/drivers/usb/dwc3/dwc3-generic.c > @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice > *dev) > return 0; > } > > -#if CONFIG_IS_ENABLED(DM_USB_GADGET) > +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && > CONFIG_IS_ENABLED(DM_USB_GADGET) Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET , since I think the DWC3 code doesn't work without DM anyway . >>> >>> Do you mean in addition to this? I do not think that alone is something >>> that will address my intention to be able to disable the gadget part of >>> the dwc3-generic driver. >>> >>> Before this patch it was possible to enable disable host or gadget by >>> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options. >>> >>> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to >>> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not >>> possible and result in the build errors reported, i.e. multiple >>> definition of dm_usb_gadget_handle_interrupts() and undefined reference >>> to dwc3_gadget_uboot_handle_interrupt(). >>> >>> After this patch I can use dm dwc2 gadget and dwc3 host without issue. >> >> Note that *DM_* symbols can be confusing. At this point, in SPL >> SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for >> host/gadget drivers. In full U-Boot, DM_USB is required for all host >> drivers and really should be enabled in all cases for DM_USB_GADGET. We >> can't enforce that at the Kconfig level because of, iirc, some of the >> armv7 part gadget drivers (more than one). But for newer and still very >> active chips, we should be using DM_USB_GADGET unconditionally in full >> U-Boot. > > The issue is that with DM_USB_GADGET we can only have one driver > providing the dm_usb_gadget_handle_interrupts() function. DM was always intended to permit multiple drivers, so this is a bug and should be fixed, e.g. by turning the interrupt handling function into a driver-specific callback. >>> >>> I fully agree, and I hope that someone can work on that separate issue >>> and that it does not block this build error fix patch. >> >> Please, let's fix this properly instead of piling up another ifdef >> workaround. I agree with Marek. > > Reworking the core usb gadget interrupt handling is unfortunately not > something I can spend any of my already limited hobbyist free time on. I understand you don't have time for this. I will try to tackle it, but I have limited time in the upcoming weeks. I will keep you posted. > > If this workaround is not acceptable I can just drop the RK3328 cleanup > and leave usb in current semi-broken state. > >> >> Since CI does pass, where is there a build error ? > > Currently there is no board with a configuration that cause build errors, > that happens when I add DM_USB_GADGET=y to RK3328 boards in a separate > series. > > Previously you have requested that I do not send usb patches as part of > a larger
[GIT PULL] Please pull u-boot-dfu-20240215
Hi Tom, Here are some developments for master including: - Fix avb_verify command with SD cards - Add u-boot-dfu maintainer tree for AB/AVB - Avb: report verified boot state based on lock state - Misc avb refactors improve code quality The CI job is at https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/19653 Thanks, Mattijs The following changes since commit 37345abb97ef0dd9c50a03b2a72617612dcae585: Prepare v2024.04-rc2 (2024-02-13 18:16:57 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-dfu.git tags/u-boot-dfu-20240215 for you to fetch changes up to e2f6270f2da844d598555702d3af4f2e3fae3f9f: doc: android: avb: sync usage details (2024-02-15 10:38:34 +0100) u-boot-dfu-20240215 - Fix avb_verify command with SD cards - Add u-boot-dfu maintainer tree for AB/AVB - Avb: report verified boot state based on lock state - Misc avb refactors improve code quality Igor Opaniuk (8): MAINTAINERS: add custodian tree info for AVB/AB common: avb_verify: don't call mmc_switch_part for SD avb: move SPDX license identifiers to the first line common: avb_verify: rework error/debug prints cmd: avb: rework prints common: avb_verify: add str_avb_io_error/str_avb_slot_error cmd: avb: rework do_avb_verify_part doc: android: avb: sync usage details MAINTAINERS| 2 + cmd/avb.c | 173 + common/avb_verify.c| 89 + doc/android/avb2.rst | 16 ++- include/avb_verify.h | 7 +- test/py/tests/test_android/test_avb.py | 3 +- 6 files changed, 180 insertions(+), 110 deletions(-)
