Re: [PATCH v2 0/5] bootstd: Add Android support

2024-06-19 Thread Mattijs Korpershoek
Hi Simon.

On mar., juin 18, 2024 at 21:03, Simon Glass  wrote:

> Hi Mattijs,
>
> On Mon, 17 Jun 2024 at 09:15, Mattijs Korpershoek
>  wrote:
>>
>> Hi Simon,
>>
>> On lun., juin 17, 2024 at 07:53, Simon Glass  wrote:
>>
>> > Hi Mattijs,
>> >
>> > On Thu, 13 Jun 2024 at 04:13, Mattijs Korpershoek
>> >  wrote:
>> >>
>> >> Android boot flow is a bit different than a regular Linux distro.
>> >> Android relies on multiple partitions in order to boot.
>> >>
>> >> A typical boot flow would be:
>> >> 1. Parse the Bootloader Control Block (BCB, misc partition)
>> >> 2. If BCB requested bootonce-bootloader, start fastboot and wait.
>> >> 3. If BCB requested recovery or normal android, run the following:
>> >>a. Get slot (A/B) from BCB
>> >>b. Run AVB (Android Verified Boot) on boot partitions
>> >>c. Load boot and vendor_boot partitions
>> >>d. Load device-tree, ramdisk and boot
>> >>
>> >> The AOSP documentation has more details at [1], [2], [3]
>> >>
>> >> This has been implemented via complex boot scripts such as [4].
>> >> However, these boot script are neither very maintainable nor generic.
>> >> Moreover, DISTRO_DEFAULTS is being deprecated [5].
>> >>
>> >> Add a generic Android bootflow implementation for bootstd.
>> >>
>> >> For this initial version, only boot image v4 is supported.
>> >>
>> >> This has been tested on sandbox using:
>> >> $ ./test/py/test.py --bd sandbox --build -k test_ut
>> >>
>> >> This has also been tested on the AM62X SK EVM using TI's Android SDK[6]
>> >> To test on TI board, the following (WIP) patch is needed as well:
>> >> https://gitlab.baylibre.com/baylibre/ti/ti-u-boot/-/commit/84cceb912bccd7cdd7f9dd69bca0e5d987a1fd04
>> >>
>> >> [1] https://source.android.com/docs/core/architecture/bootloader
>> >> [2] https://source.android.com/docs/core/architecture/partitions
>> >> [3] 
>> >> https://source.android.com/docs/core/architecture/partitions/generic-boot
>> >> [4] 
>> >> https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_android.h
>> >> [5] 
>> >> https://lore.kernel.org/r/all/20230914165615.1058529-17-...@chromium.org/
>> >> [6] 
>> >> https://software-dl.ti.com/processor-sdk-android/esd/AM62X/09_02_00/docs/android/Overview.html
>> >>
>> >> Signed-off-by: Mattijs Korpershoek 
>> >> ---
>> >> Changes in v2:
>> >> - Dropped patch 2/6 boot: android: Add image_android_get_version() (Igor)
>> >> - Fixed multi-line comment style (Igor, Simon)
>> >> - Added dependency on CMD_FASTBOOT for BOOTMETH_ANDROID (Igor)
>> >> - Fixed various resource leaks (Igor)
>> >> - Fixed bootmeth_priv dangling pointer on error cases (Igor)
>> >> - Updated test instructions in commit message for patch 6/6
>> >> - Added __weak impl of get_avendor_bootimg_addr() in patch 1 (dropped
>> >>   Igor's review because of this change)
>> >> - Added extra info in Kconfig to detail MMC limitation (Simon)
>> >> - Fixed typo Bootmethod->Bootmeth (Simon)
>> >> - Documented android_priv structure (Simon)
>> >> - Demoted various messages from printf() to log_debug (Simon)
>> >> - Fixed some lines too long (Simon)
>> >> - Added function documentation to read_slotted_partition() (Simon)
>> >> - Added some doc about avb extra_args being modified (Simon)
>> >> - Link to v1: 
>> >> https://lore.kernel.org/r/20240606-bootmeth-android-v1-0-0c69d4457...@baylibre.com
>> >>
>> >> ---
>> >> Mattijs Korpershoek (5):
>> >>   boot: android: Provide vendor_bootimg_addr in boot_get_fdt()
>> >>   bootstd: Add bootflow_iter_check_mmc() helper
>> >>   android: boot: Add set_abootimg_addr() and 
>> >> set_avendor_bootimg_addr()
>> >>   bootstd: Add a bootmeth for Android
>> >>   bootstd: Add test for bootmeth_android
>> >>
>> >>  MAINTAINERS   |   7 +
>> >>  arch/sandbox/dts/test.dts |   8 +
>> >>  boot/Kconfig  |  16 ++
>> >>  boot/Makefile |   2 +
>> >>  boot/bootflow.c   |  12 +
>> >>  boot/bootmeth_android.c   | 553 
>> >> ++

Re: [PATCH v3 1/1] usb: informative message if no controller

2024-06-18 Thread Mattijs Korpershoek
Hi Heinrich,

Thank you for the patch.

On mar., juin 18, 2024 at 11:58, Heinrich Schuchardt 
 wrote:

> The message 'No working controllers found' provides no clue that this
> refers to USB controllers.
>
> Provide a message that refers to USB. Use log_info().
>
> Signed-off-by: Heinrich Schuchardt 

Reviewed-by: Mattijs Korpershoek 

> ---
> v3:
>   plural controllers
> v2:
>   add 'found' at end of message
>   keep printf
> ---
>  drivers/usb/host/usb-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index a1cd0ad2d66..e16432a1516 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -388,7 +388,7 @@ int usb_init(void)
>  
>   /* if we were not able to find at least one working bus, bail out */
>   if (controllers_initialized == 0)
> - printf("No working controllers found\n");
> + printf("No USB controllers found\n");
>  
>   return usb_started ? 0 : -ENOENT;
>  }
> -- 
> 2.43.0


Re: [PATCH 11/11] usb: gadget: Mark dm_usb_gadget_handle_interrupts as non-weak for DM_USB_GADGET

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> The dm_usb_gadget_handle_interrupts() is not implemented by any USB
> gadget controller drivers which do enable DM_USB_GADGET anymore. Set
> the symbol as non-weak.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/udc/udc-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/udc-uclass.c 
> b/drivers/usb/gadget/udc/udc-uclass.c
> index 2320039fe3b..fbe62bbce47 100644
> --- a/drivers/usb/gadget/udc/udc-uclass.c
> +++ b/drivers/usb/gadget/udc/udc-uclass.c
> @@ -18,7 +18,7 @@ usb_gadget_generic_dev_ops(struct udevice *dev)
>   return (const struct usb_gadget_generic_ops *)dev->driver->ops;
>  }
>  
> -__weak int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> +int dm_usb_gadget_handle_interrupts(struct udevice *dev)
>  {
>   const struct usb_gadget_generic_ops *ops;
>  
> -- 
> 2.43.0


Re: [PATCH 10/11] usb: gadget: sandbox: Drop dm_usb_gadget_handle_interrupts()

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Drop dm_usb_gadget_handle_interrupts() in favor of empty default
> implementation of the same in drivers/usb/gadget/udc/udc-uclass.c .
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/host/usb-sandbox.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/usb/host/usb-sandbox.c b/drivers/usb/host/usb-sandbox.c
> index e26f0b292ed..f687fe2c430 100644
> --- a/drivers/usb/host/usb-sandbox.c
> +++ b/drivers/usb/host/usb-sandbox.c
> @@ -123,12 +123,7 @@ static int sandbox_submit_int(struct udevice *bus, 
> struct usb_device *udev,
>   return ret;
>  }
>  
> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - return 0;
> -}
> -#else
> +#if !CONFIG_IS_ENABLED(DM_USB_GADGET)
>  int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>  {
>   struct sandbox_udc *dev = this_controller;
> -- 
> 2.43.0


Re: [PATCH 08/11] usb: gadget: musb: Convert interrupt handling to usb_gadget_generic_ops

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Implement .handle_interrupts callback as a replacement for deprecated
> dm_usb_gadget_handle_interrupts() function. The new callback allows
> for each DM capable USB gadget controller driver to define its own
> IRQ handling implementation without colliding with other controller
> drivers.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/musb-new/ti-musb.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/musb-new/ti-musb.c b/drivers/usb/musb-new/ti-musb.c
> index 76e8b88369e..ec1baa9337d 100644
> --- a/drivers/usb/musb-new/ti-musb.c
> +++ b/drivers/usb/musb-new/ti-musb.c
> @@ -233,15 +233,6 @@ static int ti_musb_peripheral_of_to_plat(struct udevice 
> *dev)
>  }
>  #endif
>  
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - struct ti_musb_peripheral *priv = dev_get_priv(dev);
> -
> - priv->periph->isr(0, priv->periph);
> -
> - return 0;
> -}
> -
>  static int ti_musb_peripheral_probe(struct udevice *dev)
>  {
>   struct ti_musb_peripheral *priv = dev_get_priv(dev);
> @@ -269,12 +260,26 @@ static int ti_musb_peripheral_remove(struct udevice 
> *dev)
>   return 0;
>  }
>  
> +static int ti_musb_gadget_handle_interrupts(struct udevice *dev)
> +{
> + struct ti_musb_peripheral *priv = dev_get_priv(dev);
> +
> + priv->periph->isr(0, priv->periph);
> +
> + return 0;
> +}
> +
> +static const struct usb_gadget_generic_ops ti_musb_gadget_ops = {
> + .handle_interrupts  = ti_musb_gadget_handle_interrupts,
> +};
> +
>  U_BOOT_DRIVER(ti_musb_peripheral) = {
>   .name   = "ti-musb-peripheral",
>   .id = UCLASS_USB_GADGET_GENERIC,
>  #if CONFIG_IS_ENABLED(OF_CONTROL)
>   .of_to_plat = ti_musb_peripheral_of_to_plat,
>  #endif
> + .ops= _musb_gadget_ops,
>   .probe = ti_musb_peripheral_probe,
>   .remove = ti_musb_peripheral_remove,
>   .ops= _usb_ops,
> -- 
> 2.43.0


Re: [PATCH 09/11] usb: gadget: ux500: Convert interrupt handling to usb_gadget_generic_ops

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Implement .handle_interrupts callback as a replacement for deprecated
> dm_usb_gadget_handle_interrupts() function. The new callback allows
> for each DM capable USB gadget controller driver to define its own
> IRQ handling implementation without colliding with other controller
> drivers.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

nitpick below (up to you if you want to fix it)

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/musb-new/ux500.c | 22 ++
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/musb-new/ux500.c b/drivers/usb/musb-new/ux500.c
> index 6b4ef3c8578..89dd75b7d05 100644
> --- a/drivers/usb/musb-new/ux500.c
> +++ b/drivers/usb/musb-new/ux500.c
> @@ -91,14 +91,6 @@ static const struct musb_platform_ops ux500_musb_ops = {
>   .disable= ux500_musb_disable,
>  };
>  
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - struct ux500_glue *glue = dev_get_priv(dev);
> -
> - glue->mdata.host->isr(0, glue->mdata.host);
> - return 0;
> -}
> -
>  static int ux500_musb_probe(struct udevice *dev)
>  {
>  #ifdef CONFIG_USB_MUSB_HOST
> @@ -155,6 +147,19 @@ static int ux500_musb_remove(struct udevice *dev)
>   return 0;
>  }
>  
> +static int ux500_gadget_handle_interrupts(struct udevice *dev)
> +{
> + struct ux500_glue *glue = dev_get_priv(dev);
> +
> + glue->mdata.host->isr(0, glue->mdata.host);
> +
> + return 0;
> +}
> +
> +static const struct usb_gadget_generic_ops ux500_gadget_ops = {
> + .handle_interrupts  = ux500_gadget_handle_interrupts,
> +};
> +
>  static const struct udevice_id ux500_musb_ids[] = {
>   { .compatible = "stericsson,db8500-musb" },
>   { }
> @@ -168,6 +173,7 @@ U_BOOT_DRIVER(ux500_musb) = {
>   .id = UCLASS_USB_GADGET_GENERIC,
>  #endif
>   .of_match   = ux500_musb_ids,
> + .ops= _gadget_ops,

In case of CONFIG_USB_MUSB_HOST=y, ops gets redefined below.
Can we move this assignment below
```
.id = UCLASS_USB_GADGET_GENERIC,
```
?

>   .probe  = ux500_musb_probe,
>   .remove = ux500_musb_remove,
>  #ifdef CONFIG_USB_MUSB_HOST
> -- 
> 2.43.0


Re: [PATCH 04/11] usb: gadget: dwc3: Convert interrupt handling to usb_gadget_generic_ops

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Implement .handle_interrupts callback as a replacement for deprecated
> dm_usb_gadget_handle_interrupts() function. The new callback allows
> for each DM capable USB gadget controller driver to define its own
> IRQ handling implementation without colliding with other controller
> drivers.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/dwc3/dwc3-generic.c| 23 ++-
>  drivers/usb/dwc3/dwc3-layerscape.c | 21 +
>  2 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
> index 8db678eb85d..731ede2fead 100644
> --- a/drivers/usb/dwc3/dwc3-generic.c
> +++ b/drivers/usb/dwc3/dwc3-generic.c
> @@ -194,34 +194,39 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>  }
>  
>  #if CONFIG_IS_ENABLED(DM_USB_GADGET)
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> +static int dwc3_generic_peripheral_probe(struct udevice *dev)
>  {
>   struct dwc3_generic_priv *priv = dev_get_priv(dev);
> - struct dwc3 *dwc3 = >dwc3;
>  
> - dwc3_gadget_uboot_handle_interrupt(dwc3);
> -
> - return 0;
> + return dwc3_generic_probe(dev, priv);
>  }
>  
> -static int dwc3_generic_peripheral_probe(struct udevice *dev)
> +static int dwc3_generic_peripheral_remove(struct udevice *dev)
>  {
>   struct dwc3_generic_priv *priv = dev_get_priv(dev);
>  
> - return dwc3_generic_probe(dev, priv);
> + return dwc3_generic_remove(dev, priv);
>  }
>  
> -static int dwc3_generic_peripheral_remove(struct udevice *dev)
> +static int dwc3_gadget_handle_interrupts(struct udevice *dev)
>  {
>   struct dwc3_generic_priv *priv = dev_get_priv(dev);
> + struct dwc3 *dwc3 = >dwc3;
>  
> - return dwc3_generic_remove(dev, priv);
> + dwc3_gadget_uboot_handle_interrupt(dwc3);
> +
> + return 0;
>  }
>  
> +static const struct usb_gadget_generic_ops dwc3_gadget_ops = {
> + .handle_interrupts  = dwc3_gadget_handle_interrupts,
> +};
> +
>  U_BOOT_DRIVER(dwc3_generic_peripheral) = {
>   .name   = "dwc3-generic-peripheral",
>   .id = UCLASS_USB_GADGET_GENERIC,
>   .of_to_plat = dwc3_generic_of_to_plat,
> + .ops= _gadget_ops,
>   .probe = dwc3_generic_peripheral_probe,
>   .remove = dwc3_generic_peripheral_remove,
>   .priv_auto  = sizeof(struct dwc3_generic_priv),
> diff --git a/drivers/usb/dwc3/dwc3-layerscape.c 
> b/drivers/usb/dwc3/dwc3-layerscape.c
> index ff83bf71e89..108b44c67eb 100644
> --- a/drivers/usb/dwc3/dwc3-layerscape.c
> +++ b/drivers/usb/dwc3/dwc3-layerscape.c
> @@ -99,33 +99,38 @@ static int dwc3_layerscape_of_to_plat(struct udevice *dev)
>  }
>  
>  #if CONFIG_IS_ENABLED(DM_USB_GADGET)
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> +static int dwc3_layerscape_peripheral_probe(struct udevice *dev)
>  {
>   struct dwc3_layerscape_priv *priv = dev_get_priv(dev);
>  
> - dwc3_gadget_uboot_handle_interrupt(>dwc3);
> -
> - return 0;
> + return dwc3_layerscape_probe(dev, priv);
>  }
>  
> -static int dwc3_layerscape_peripheral_probe(struct udevice *dev)
> +static int dwc3_layerscape_peripheral_remove(struct udevice *dev)
>  {
>   struct dwc3_layerscape_priv *priv = dev_get_priv(dev);
>  
> - return dwc3_layerscape_probe(dev, priv);
> + return dwc3_layerscape_remove(dev, priv);
>  }
>  
> -static int dwc3_layerscape_peripheral_remove(struct udevice *dev)
> +static int dwc3_layerscape_gadget_handle_interrupts(struct udevice *dev)
>  {
>   struct dwc3_layerscape_priv *priv = dev_get_priv(dev);
>  
> - return dwc3_layerscape_remove(dev, priv);
> + dwc3_gadget_uboot_handle_interrupt(>dwc3);
> +
> + return 0;
>  }
>  
> +static const struct usb_gadget_generic_ops dwc3_layerscape_gadget_ops = {
> + .handle_interrupts  = dwc3_layerscape_gadget_handle_interrupts,
> +};
> +
>  U_BOOT_DRIVER(dwc3_layerscape_peripheral) = {
>   .name   = "dwc3-layerscape-peripheral",
>   .id = UCLASS_USB_GADGET_GENERIC,
>   .of_to_plat = dwc3_layerscape_of_to_plat,
> + .ops= _layerscape_gadget_ops,
>   .probe = dwc3_layerscape_peripheral_probe,
>   .remove = dwc3_layerscape_peripheral_remove,
>   .priv_auto  = sizeof(struct dwc3_layerscape_priv),
> -- 
> 2.43.0


Re: [PATCH 06/11] usb: gadget: mtu3: Convert interrupt handling to usb_gadget_generic_ops

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Implement .handle_interrupts callback as a replacement for deprecated
> dm_usb_gadget_handle_interrupts() function. The new callback allows
> for each DM capable USB gadget controller driver to define its own
> IRQ handling implementation without colliding with other controller
> drivers.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/mtu3/mtu3_plat.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/mtu3/mtu3_plat.c b/drivers/usb/mtu3/mtu3_plat.c
> index ca86b58dfcd..f8e14eabfb2 100644
> --- a/drivers/usb/mtu3/mtu3_plat.c
> +++ b/drivers/usb/mtu3/mtu3_plat.c
> @@ -223,15 +223,6 @@ static const struct udevice_id ssusb_of_match[] = {
>  };
>  
>  #if CONFIG_IS_ENABLED(DM_USB_GADGET)
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - struct mtu3 *mtu = dev_get_priv(dev);
> -
> - mtu3_irq(0, mtu);
> -
> - return 0;
> -}
> -
>  static int mtu3_gadget_probe(struct udevice *dev)
>  {
>   struct ssusb_mtk *ssusb = dev_to_ssusb(dev->parent);
> @@ -250,10 +241,24 @@ static int mtu3_gadget_remove(struct udevice *dev)
>   return 0;
>  }
>  
> +static int mtu3_gadget_handle_interrupts(struct udevice *dev)
> +{
> + struct mtu3 *mtu = dev_get_priv(dev);
> +
> + mtu3_irq(0, mtu);
> +
> + return 0;
> +}
> +
> +static const struct usb_gadget_generic_ops mtu3_gadget_ops = {
> + .handle_interrupts  = mtu3_gadget_handle_interrupts,
> +};
> +
>  U_BOOT_DRIVER(mtu3_peripheral) = {
>   .name = "mtu3-peripheral",
>   .id = UCLASS_USB_GADGET_GENERIC,
>   .of_match = ssusb_of_match,
> + .ops = _gadget_ops,
>   .probe = mtu3_gadget_probe,
>   .remove = mtu3_gadget_remove,
>   .priv_auto  = sizeof(struct mtu3),
> -- 
> 2.43.0


Re: [PATCH 05/11] usb: gadget: max3420: Convert interrupt handling to usb_gadget_generic_ops

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Implement .handle_interrupts callback as a replacement for deprecated
> dm_usb_gadget_handle_interrupts() function. The new callback allows
> for each DM capable USB gadget controller driver to define its own
> IRQ handling implementation without colliding with other controller
> drivers.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/max3420_udc.c | 19 ---
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/max3420_udc.c 
> b/drivers/usb/gadget/max3420_udc.c
> index 5a227c0ffd9..557a1f0644e 100644
> --- a/drivers/usb/gadget/max3420_udc.c
> +++ b/drivers/usb/gadget/max3420_udc.c
> @@ -808,13 +808,6 @@ static void max3420_setup_spi(struct max3420_udc *udc)
>   spi_wr8(udc, MAX3420_REG_PINCTL, bFDUPSPI);
>  }
>  
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - struct max3420_udc *udc = dev_get_priv(dev);
> -
> - return max3420_irq(udc);
> -}
> -
>  static int max3420_udc_probe(struct udevice *dev)
>  {
>   struct max3420_udc *udc = dev_get_priv(dev);
> @@ -859,6 +852,17 @@ static int max3420_udc_remove(struct udevice *dev)
>   return 0;
>  }
>  
> +static int max3420_gadget_handle_interrupts(struct udevice *dev)
> +{
> + struct max3420_udc *udc = dev_get_priv(dev);
> +
> + return max3420_irq(udc);
> +}
> +
> +static const struct usb_gadget_generic_ops max3420_gadget_ops = {
> + .handle_interrupts  = max3420_gadget_handle_interrupts,
> +};
> +
>  static const struct udevice_id max3420_ids[] = {
>   { .compatible = "maxim,max3421-udc" },
>   { }
> @@ -868,6 +872,7 @@ U_BOOT_DRIVER(max3420_generic_udc) = {
>   .name = "max3420-udc",
>   .id = UCLASS_USB_GADGET_GENERIC,
>   .of_match = max3420_ids,
> + .ops = _gadget_ops,
>   .probe = max3420_udc_probe,
>   .remove = max3420_udc_remove,
>   .priv_auto  = sizeof(struct max3420_udc),
> -- 
> 2.43.0


Re: [PATCH 03/11] usb: gadget: dwc2: Convert interrupt handling to usb_gadget_generic_ops

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Implement .handle_interrupts callback as a replacement for deprecated
> dm_usb_gadget_handle_interrupts() function. The new callback allows
> for each DM capable USB gadget controller driver to define its own
> IRQ handling implementation without colliding with other controller
> drivers.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/dwc2_udc_otg.c | 20 +++-
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c 
> b/drivers/usb/gadget/dwc2_udc_otg.c
> index 6bd395a6235..7e9dd6f4268 100644
> --- a/drivers/usb/gadget/dwc2_udc_otg.c
> +++ b/drivers/usb/gadget/dwc2_udc_otg.c
> @@ -941,11 +941,6 @@ int dwc2_udc_handle_interrupt(void)
>   return 0;
>  }
>  
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - return dwc2_udc_handle_interrupt();
> -}
> -
>  #if CONFIG_IS_ENABLED(DM_USB_GADGET)
>  struct dwc2_priv_data {
>   struct clk_bulk clks;
> @@ -1173,6 +1168,15 @@ static int dwc2_udc_otg_remove(struct udevice *dev)
>   return dm_scan_fdt_dev(dev);
>  }
>  
> +static int dwc2_gadget_handle_interrupts(struct udevice *dev)
> +{
> + return dwc2_udc_handle_interrupt();
> +}
> +
> +static const struct usb_gadget_generic_ops dwc2_gadget_ops = {
> + .handle_interrupts  = dwc2_gadget_handle_interrupts,
> +};
> +
>  static const struct udevice_id dwc2_udc_otg_ids[] = {
>   { .compatible = "snps,dwc2" },
>   { .compatible = "brcm,bcm2835-usb" },
> @@ -1185,6 +1189,7 @@ U_BOOT_DRIVER(dwc2_udc_otg) = {
>   .name   = "dwc2-udc-otg",
>   .id = UCLASS_USB_GADGET_GENERIC,
>   .of_match = dwc2_udc_otg_ids,
> + .ops= _gadget_ops,
>   .of_to_plat = dwc2_udc_otg_of_to_plat,
>   .probe = dwc2_udc_otg_probe,
>   .remove = dwc2_udc_otg_remove,
> @@ -1200,4 +1205,9 @@ int dwc2_udc_B_session_valid(struct udevice *dev)
>  
>   return readl(_reg->gotgctl) & B_SESSION_VALID;
>  }
> +#else
> +int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> +{
> + return dwc2_udc_handle_interrupt();
> +}
>  #endif /* CONFIG_IS_ENABLED(DM_USB_GADGET) */
> -- 
> 2.43.0


Re: [PATCH 02/11] usb: gadget: cdns3: Convert interrupt handling to usb_gadget_generic_ops

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Implement .handle_interrupts callback as a replacement for deprecated
> dm_usb_gadget_handle_interrupts() function. The new callback allows
> for each DM capable USB gadget controller driver to define its own
> IRQ handling implementation without colliding with other controller
> drivers.
>
> Keep the dm_usb_gadget_handle_interrupts() in this driver for non-DM
> case for now, until this driver gets fully converted to DM USB gadget.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/cdns3/core.c  | 24 
>  drivers/usb/cdns3/gadget-export.h |  2 ++
>  drivers/usb/cdns3/gadget.c| 11 +--
>  3 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index b4e931646b8..cbe06a9e7b6 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -462,15 +463,38 @@ static int cdns3_gadget_remove(struct udevice *dev)
>   return cdns3_remove(cdns);
>  }
>  
> +static int cdns3_gadget_handle_interrupts(struct udevice *dev)
> +{
> + struct cdns3 *cdns = dev_get_priv(dev);
> +
> + cdns3_gadget_uboot_handle_interrupt(cdns);
> +
> + return 0;
> +}
> +
> +static const struct usb_gadget_generic_ops cdns3_gadget_ops = {
> + .handle_interrupts  = cdns3_gadget_handle_interrupts,
> +};
> +
>  U_BOOT_DRIVER(cdns_usb3_peripheral) = {
>   .name   = "cdns-usb3-peripheral",
>   .id = UCLASS_USB_GADGET_GENERIC,
>   .of_match = cdns3_ids,
> + .ops= _gadget_ops,
>   .probe = cdns3_gadget_probe,
>   .remove = cdns3_gadget_remove,
>   .priv_auto  = sizeof(struct cdns3_gadget_priv),
>   .flags = DM_FLAG_ALLOC_PRIV_DMA,
>  };
> +#else
> +int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> +{
> + struct cdns3 *cdns = dev_get_priv(dev);
> +
> + cdns3_gadget_uboot_handle_interrupt(cdns);
> +
> + return 0;
> +}
>  #endif
>  
>  #if defined(CONFIG_SPL_USB_HOST) || \
> diff --git a/drivers/usb/cdns3/gadget-export.h 
> b/drivers/usb/cdns3/gadget-export.h
> index 577469eee96..b3fd7c53039 100644
> --- a/drivers/usb/cdns3/gadget-export.h
> +++ b/drivers/usb/cdns3/gadget-export.h
> @@ -25,4 +25,6 @@ static inline void cdns3_gadget_exit(struct cdns3 *cdns) { }
>  
>  #endif
>  
> +void cdns3_gadget_uboot_handle_interrupt(struct cdns3 *cdns);
> +
>  #endif /* __LINUX_CDNS3_GADGET_EXPORT */
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index d11175dc5b6..32b2c412068 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -2755,19 +2755,10 @@ int cdns3_gadget_init(struct cdns3 *cdns)
>   *
>   * Handles ep0 and gadget interrupt
>   */
> -static void cdns3_gadget_uboot_handle_interrupt(struct cdns3 *cdns)
> +void cdns3_gadget_uboot_handle_interrupt(struct cdns3 *cdns)
>  {
>   int ret = cdns3_device_irq_handler(0, cdns);
>  
>   if (ret == IRQ_WAKE_THREAD)
>   cdns3_device_thread_irq_handler(0, cdns);
>  }
> -
> -int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> -{
> - struct cdns3 *cdns = dev_get_priv(dev);
> -
> - cdns3_gadget_uboot_handle_interrupt(cdns);
> -
> - return 0;
> -}
> -- 
> 2.43.0


Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

2024-06-18 Thread Mattijs Korpershoek
Hi Jonas,

On sam., mars 02, 2024 at 14:00, Jonas Karlman  wrote:

[...]

>> 
>> I will keep you posted.
>
> Thanks, much appreciated!
>
> Please also keep in mind that changing the interrupt handling probably
> only fixes the second of the two build errors reported and fixed by this
> patch.
>
> Trying to build with following will trigger the first build error, and
> should not change because use of dm_usb_gadget_handle_interrupts() is
> reworked.
>
> CONFIG_DM_USB_GADGET=y
> CONFIG_USB_DWC3=y
> # CONFIG_USB_DWC3_GADGET is not set
> CONFIG_USB_DWC3_GENERIC=y
> CONFIG_USB_GADGET=y
>
> E.g to only include host part of dwc3 and gadget from another driver,
> to i.e. save on binary size, produce following build error:
>
> aarch64-linux-gnu-ld.bfd: drivers/usb/dwc3/dwc3-generic.o: in function 
> `dm_usb_gadget_handle_interrupts':
> drivers/usb/dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>   undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>
> I guess force select USB_DWC3_GADGET for USB_DWC3_GENERIC would make
> that build error disappear, and increase binary size as a result.
>
> For my RK3328 series [1] I will just revert to use USB_XHCI_DWC3 instead
> of using USB_DWC3_GENERIC on boards that enable peripheral use of otg
> port.
>
> [1] https://patchwork.ozlabs.org/patch/1904779/

Marek ended up doing this rework.
It's available for review here, if you want to have a look:
https://lore.kernel.org/all/20240614005309.34433-1-marek.vasut+rene...@mailbox.org/

>
> Regards,
> Jonas
>
>> 
>>>

[...]


Re: [PATCH 01/11] usb: gadget: Introduce handle_interrupts ops to USB_GADGET_GENERIC uclass

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Introduce .ops for USB_GADGET_GENERIC uclass. The first new ops is
> .handle_interrupts which must be implemented by DM capable USB gadget
> controller drivers and must implement interrupt handling similar to
> dm_usb_gadget_handle_interrupts(). This patch currently provides weak
> dm_usb_gadget_handle_interrupts() implementation which is overriden by
> the drivers, but this will be removed once conversion to handle_interrupts
> callback is complete.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/udc/udc-uclass.c | 24 
>  include/linux/usb/gadget.h  |  8 
>  2 files changed, 32 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/udc-uclass.c 
> b/drivers/usb/gadget/udc/udc-uclass.c
> index 5dc23a55bb5..2320039fe3b 100644
> --- a/drivers/usb/gadget/udc/udc-uclass.c
> +++ b/drivers/usb/gadget/udc/udc-uclass.c
> @@ -12,6 +12,25 @@
>  #include 
>  
>  #if CONFIG_IS_ENABLED(DM_USB_GADGET)
> +static inline const struct usb_gadget_generic_ops *
> +usb_gadget_generic_dev_ops(struct udevice *dev)
> +{
> + return (const struct usb_gadget_generic_ops *)dev->driver->ops;
> +}
> +
> +__weak int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> +{
> + const struct usb_gadget_generic_ops *ops;
> +
> + ops = usb_gadget_generic_dev_ops(dev);
> + if (!ops)
> + return -EFAULT;
> + if (!ops->handle_interrupts)
> + return -ENOSYS;
> +
> + return ops->handle_interrupts(dev);
> +}
> +
>  int udc_device_get_by_index(int index, struct udevice **udev)
>  {
>   struct udevice *dev = NULL;
> @@ -54,6 +73,11 @@ int udc_device_put(struct udevice *udev)
>  {
>   return board_usb_cleanup(legacy_index, USB_INIT_DEVICE);
>  }
> +
> +__weak int dm_usb_gadget_handle_interrupts(struct udevice *dev)
> +{
> + return 0;
> +}
>  #endif
>  
>  #if CONFIG_IS_ENABLED(DM)
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 36572be89e6..cf2161603d6 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -970,6 +970,14 @@ extern void usb_ep_autoconfig_reset(struct usb_gadget *);
>  
>  extern int dm_usb_gadget_handle_interrupts(struct udevice *);
>  
> +/**
> + * struct usb_gadget_generic_ops - The functions that a gadget driver must 
> implement.
> + * @handle_interrupts: Handle UDC interrupts.
> + */
> +struct usb_gadget_generic_ops {
> + int (*handle_interrupts)(struct udevice *udevice);
> +};
> +
>  /**
>   * udc_device_get_by_index() - Get UDC udevice by index
>   * @index: UDC device index
> -- 
> 2.43.0


Re: [PATCH 00/11] usb: gadget: Introduce handle_interrupts ops to USB_GADGET_GENERIC uclass

2024-06-18 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the series.

On ven., juin 14, 2024 at 02:51, Marek Vasut  
wrote:

> Introduce .ops for USB_GADGET_GENERIC uclass. The first new ops is
> .handle_interrupts which must be implemented by DM capable USB gadget
> controller drivers and must implement interrupt handling similar to
> dm_usb_gadget_handle_interrupts(). For DM USB gadget drivers this is
> a replacement for dm_usb_gadget_handle_interrupts(). Convert the DM
> USB gadget drivers to this new ops instead.
>
> DEPENDS: https://patchwork.ozlabs.org/project/uboot/list/?series=410150
>
> Marek Vasut (11):
>   usb: gadget: Introduce handle_interrupts ops to USB_GADGET_GENERIC
> uclass
>   usb: gadget: cdns3: Convert interrupt handling to
> usb_gadget_generic_ops
>   usb: gadget: dwc2: Convert interrupt handling to
> usb_gadget_generic_ops
>   usb: gadget: dwc3: Convert interrupt handling to
> usb_gadget_generic_ops
>   usb: gadget: max3420: Convert interrupt handling to
> usb_gadget_generic_ops
>   usb: gadget: mtu3: Convert interrupt handling to
> usb_gadget_generic_ops
>   usb: gadget: omap2430: Convert interrupt handling to
> usb_gadget_generic_ops
>   usb: gadget: musb: Convert interrupt handling to
> usb_gadget_generic_ops
>   usb: gadget: ux500: Convert interrupt handling to
> usb_gadget_generic_ops
>   usb: gadget: sandbox: Drop dm_usb_gadget_handle_interrupts()
>   usb: gadget: Mark dm_usb_gadget_handle_interrupts as non-weak for
> DM_USB_GADGET
>
>  drivers/usb/cdns3/core.c| 24 
>  drivers/usb/cdns3/gadget-export.h   |  2 ++
>  drivers/usb/cdns3/gadget.c  | 11 +--
>  drivers/usb/dwc3/dwc3-generic.c | 23 ++-
>  drivers/usb/dwc3/dwc3-layerscape.c  | 21 +
>  drivers/usb/gadget/dwc2_udc_otg.c   | 20 +++-
>  drivers/usb/gadget/max3420_udc.c| 19 ---
>  drivers/usb/gadget/udc/udc-uclass.c | 24 
>  drivers/usb/host/usb-sandbox.c  |  7 +--
>  drivers/usb/mtu3/mtu3_plat.c| 23 ++-
>  drivers/usb/musb-new/omap2430.c | 26 --
>  drivers/usb/musb-new/ti-musb.c  | 23 ++-
>  drivers/usb/musb-new/ux500.c| 22 ++
>  include/linux/usb/gadget.h  |  8 
>  14 files changed, 172 insertions(+), 81 deletions(-)

Tested on Khadas vim3 using fastboot, ums and usb storage scanning.

Tested-by: Mattijs Korpershoek  # vim3

>
> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
>
> -- 
> 2.43.0


Re: [PATCH v2 0/5] bootstd: Add Android support

2024-06-17 Thread Mattijs Korpershoek
Hi Simon,

On lun., juin 17, 2024 at 07:53, Simon Glass  wrote:

> Hi Mattijs,
>
> On Thu, 13 Jun 2024 at 04:13, Mattijs Korpershoek
>  wrote:
>>
>> Android boot flow is a bit different than a regular Linux distro.
>> Android relies on multiple partitions in order to boot.
>>
>> A typical boot flow would be:
>> 1. Parse the Bootloader Control Block (BCB, misc partition)
>> 2. If BCB requested bootonce-bootloader, start fastboot and wait.
>> 3. If BCB requested recovery or normal android, run the following:
>>a. Get slot (A/B) from BCB
>>b. Run AVB (Android Verified Boot) on boot partitions
>>c. Load boot and vendor_boot partitions
>>d. Load device-tree, ramdisk and boot
>>
>> The AOSP documentation has more details at [1], [2], [3]
>>
>> This has been implemented via complex boot scripts such as [4].
>> However, these boot script are neither very maintainable nor generic.
>> Moreover, DISTRO_DEFAULTS is being deprecated [5].
>>
>> Add a generic Android bootflow implementation for bootstd.
>>
>> For this initial version, only boot image v4 is supported.
>>
>> This has been tested on sandbox using:
>> $ ./test/py/test.py --bd sandbox --build -k test_ut
>>
>> This has also been tested on the AM62X SK EVM using TI's Android SDK[6]
>> To test on TI board, the following (WIP) patch is needed as well:
>> https://gitlab.baylibre.com/baylibre/ti/ti-u-boot/-/commit/84cceb912bccd7cdd7f9dd69bca0e5d987a1fd04
>>
>> [1] https://source.android.com/docs/core/architecture/bootloader
>> [2] https://source.android.com/docs/core/architecture/partitions
>> [3] https://source.android.com/docs/core/architecture/partitions/generic-boot
>> [4] 
>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_android.h
>> [5] https://lore.kernel.org/r/all/20230914165615.1058529-17-...@chromium.org/
>> [6] 
>> https://software-dl.ti.com/processor-sdk-android/esd/AM62X/09_02_00/docs/android/Overview.html
>>
>> Signed-off-by: Mattijs Korpershoek 
>> ---
>> Changes in v2:
>> - Dropped patch 2/6 boot: android: Add image_android_get_version() (Igor)
>> - Fixed multi-line comment style (Igor, Simon)
>> - Added dependency on CMD_FASTBOOT for BOOTMETH_ANDROID (Igor)
>> - Fixed various resource leaks (Igor)
>> - Fixed bootmeth_priv dangling pointer on error cases (Igor)
>> - Updated test instructions in commit message for patch 6/6
>> - Added __weak impl of get_avendor_bootimg_addr() in patch 1 (dropped
>>   Igor's review because of this change)
>> - Added extra info in Kconfig to detail MMC limitation (Simon)
>> - Fixed typo Bootmethod->Bootmeth (Simon)
>> - Documented android_priv structure (Simon)
>> - Demoted various messages from printf() to log_debug (Simon)
>> - Fixed some lines too long (Simon)
>> - Added function documentation to read_slotted_partition() (Simon)
>> - Added some doc about avb extra_args being modified (Simon)
>> - Link to v1: 
>> https://lore.kernel.org/r/20240606-bootmeth-android-v1-0-0c69d4457...@baylibre.com
>>
>> ---
>> Mattijs Korpershoek (5):
>>   boot: android: Provide vendor_bootimg_addr in boot_get_fdt()
>>   bootstd: Add bootflow_iter_check_mmc() helper
>>   android: boot: Add set_abootimg_addr() and set_avendor_bootimg_addr()
>>   bootstd: Add a bootmeth for Android
>>   bootstd: Add test for bootmeth_android
>>
>>  MAINTAINERS   |   7 +
>>  arch/sandbox/dts/test.dts |   8 +
>>  boot/Kconfig  |  16 ++
>>  boot/Makefile |   2 +
>>  boot/bootflow.c   |  12 +
>>  boot/bootmeth_android.c   | 553 
>> ++
>>  boot/bootmeth_android.h   |  29 +++
>>  boot/image-android.c  |   5 +
>>  boot/image-fdt.c  |   2 +-
>>  cmd/abootimg.c|  10 +
>>  configs/sandbox_defconfig |   2 +-
>>  doc/develop/bootstd.rst   |   6 +
>>  include/bootflow.h|   9 +
>>  include/image.h   |  14 ++
>>  test/boot/bootflow.c  |  65 +-
>>  test/py/tests/test_ut.py  |  76 +++
>>  16 files changed, 811 insertions(+), 5 deletions(-)
>> ---
>> base-commit: f9886bc60f42d5bcfcfa4e474af7dc230400b6be
>> change-id: 20240605-bootmeth-android-bfc8596e9367
>>
>> Best regards,
>> --
>> Mattijs Korpershoek 
>>
>
> Thinking about this, I believe we should start having docs about the
> individual bootmeths themselves.

Yes.

>
> Can you add a section about your new bootmeth? I will come up with a
> patch for the others that I know about. Perhaps
> doc/develop/bootstd.rst would be a suitable place for now?

Yes I can add a section. I would have preferred to have an example to
work from there, but I can start writing docs as well.

I'm leaving on vacation soon (without computer), so I'll be able to
send a v3 with docs included in at earliest a 2-3 weeks from now.

If you make a patch for the other bootmeths in the mean-time, please cc
me so that I can help review and have an example for Android.

Thanks!
Mattijs

>
> Regards,
> Simon


Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

2024-06-17 Thread Mattijs Korpershoek
Hi Heinrich,

Thank you for your review.

On dim., juin 16, 2024 at 09:38, Heinrich Schuchardt  wrote:

> On 6/4/24 17:15, Mattijs Korpershoek wrote:
>> According to [1], we should use bootmeth when describing the
>> struct bootmeth:
>>
>> """
>> For version 2, a new naming scheme is used as above:
>>
>>  - bootdev is used instead of bootdevice, because 'device' is overused,
>>  is everywhere in U-Boot, can be confused with udevice
>
> Boot devices are udevices though they don't relate to hardware but to an
> abstract concept.
>
> bootdev is just an abbreviation. This does not make the meaning any clearer.

Per my understanding, the name for this concept is "bootdev", not
"boot device", see:

https://docs.u-boot.org/en/latest/develop/bootstd.html#introduction

>
>>  - bootmeth - because 'method' is too vanilla, appears 1300 times in
>>  U-Boot
>> """
>
> Avoiding abbreviations like bootdev and bootmeth improved readability.

The above paragraph is quoted from email [1].
In this email, Simon made the choice to use bootmeth and bootdev
when pushing the initial implementation.

This patch just corrects the places where the older terminology
(bootmethod, bootdevice) was still used.

So i'm a bit confused on why this patch got rejected.
Is it preferable to keep two terminologies for the same concept?

Thanks
Mattijs


[1] https://lore.kernel.org/u-boot/20211023232635.9195-1-...@chromium.org/

>
> Best regards
>
> Heinrich
>
>>
>> Replace all occurences in various comments for consistency.
>>
>> [1] https://lore.kernel.org/u-boot/20211023232635.9195-1-...@chromium.org/
>> Signed-off-by: Mattijs Korpershoek 
>> ---
>>   board/sandbox/sandbox.env |  2 +-
>>   boot/bootmeth-uclass.c|  2 +-
>>   include/bootmeth.h| 30 +++---
>>   include/extlinux.h|  2 +-
>>   test/boot/bootflow.c  |  2 +-
>>   test/boot/bootmeth.c  |  6 +++---
>>   6 files changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/board/sandbox/sandbox.env b/board/sandbox/sandbox.env
>> index a2c19702d64d..564dce78a898 100644
>> --- a/board/sandbox/sandbox.env
>> +++ b/board/sandbox/sandbox.env
>> @@ -10,7 +10,7 @@ eth6addr=02:00:11:22:33:47
>>   ipaddr=192.0.2.1
>>
>>   /*
>> - * These are used for distro boot which is not supported. But once 
>> bootmethod
>> + * These are used for distro boot which is not supported. But once bootmeth
>>* is provided these will be used again.
>>*/
>>   bootm_size=0x1000
>> diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
>> index 1d157d54dbdd..e3475f46b34c 100644
>> --- a/boot/bootmeth-uclass.c
>> +++ b/boot/bootmeth-uclass.c
>> @@ -167,7 +167,7 @@ int bootmeth_setup_iter_order(struct bootflow_iter 
>> *iter, bool include_global)
>>  if (pass)
>>  iter->first_glob_method = upto;
>>  /*
>> - * Get a list of bootmethods, in seq order (i.e. using
>> + * Get a list of bootmeths, in seq order (i.e. using
>>   * aliases). There may be gaps so try to count up high
>>   * enough to find them all.
>>   */
>> diff --git a/include/bootmeth.h b/include/bootmeth.h
>> index 529c4d813d82..2570d9593d49 100644
>> --- a/include/bootmeth.h
>> +++ b/include/bootmeth.h
>> @@ -47,7 +47,7 @@ struct bootmeth_ops {
>>   * This may involve reading state from the system, e.g. some data in
>>   * the firmware area.
>>   *
>> - * @dev:Bootmethod device to check
>> + * @dev:Bootmeth device to check
>>   * @buf:Buffer to place the info in (terminator must fit)
>>   * @maxsize:Size of buffer
>>   * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if
>> @@ -74,7 +74,7 @@ struct bootmeth_ops {
>>   *
>>   * It may update only the flags in @iter
>>   *
>> - * @dev:Bootmethod device to check against
>> + * @dev:Bootmeth device to check against
>>   * @iter:   On entry, provides bootdev, hwpart, part
>>   * Return: 0 if OK, -ENOTSUPP if this bootdev is not supported
>>   */
>> @@ -83,7 +83,7 @@ struct bootmeth_ops {
>>  /**
>>   * read_bootflow() - read a bootflow for a device
>>   *
>> - * @dev:Bootmethod device to use
>> + * @dev:Bootmeth de

Re: [PATCH v2 0/5] bootstd: Add Android support

2024-06-14 Thread Mattijs Korpershoek
Hi Guillaume,

Thank you for testing.

On ven., juin 14, 2024 at 11:53, Guillaume LA ROQUE  
wrote:

> Hi,
>
> i apply  patch series with commit you give in cover letter and test on 
> TI AM62S-SK board.
> Android boot properly , just with a small changes in uboot eenv
>
> setenv vendor_boot_comp_addr_r 0xd000

I see.

Thank you for sharing this.

I can confirm that this is indeed needed on next since
da3447d09fa0 ("android: Fix ramdisk loading for bootimage v3+").

Will take that into account when sending board support using bootmeth_android

>
> this changes is need link to patch done by roman on ramdisk and vendor 
> boot loading.
>
> so you can add for this series:
>
> Tested-by: Guillaume La Roque 
>
>

[...]


[PATCH v2 2/5] bootstd: Add bootflow_iter_check_mmc() helper

2024-06-13 Thread Mattijs Korpershoek
Some bootflows might be able to only boot from MMC devices.

Add a helper function these bootflows can use.

Reviewed-by: Igor Opaniuk 
Signed-off-by: Mattijs Korpershoek 
---
 boot/bootflow.c| 12 
 include/bootflow.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/boot/bootflow.c b/boot/bootflow.c
index 9aa3179c3881..59d77d2385f4 100644
--- a/boot/bootflow.c
+++ b/boot/bootflow.c
@@ -575,6 +575,18 @@ int bootflow_iter_check_blk(const struct bootflow_iter 
*iter)
return -ENOTSUPP;
 }
 
+int bootflow_iter_check_mmc(const struct bootflow_iter *iter)
+{
+   const struct udevice *media = dev_get_parent(iter->dev);
+   enum uclass_id id = device_get_uclass_id(media);
+
+   log_debug("uclass %d: %s\n", id, uclass_get_name(id));
+   if (id == UCLASS_MMC)
+   return 0;
+
+   return -ENOTSUPP;
+}
+
 int bootflow_iter_check_sf(const struct bootflow_iter *iter)
 {
const struct udevice *media = dev_get_parent(iter->dev);
diff --git a/include/bootflow.h b/include/bootflow.h
index 080ee8501225..6058ddd89b16 100644
--- a/include/bootflow.h
+++ b/include/bootflow.h
@@ -407,6 +407,15 @@ void bootflow_remove(struct bootflow *bflow);
  */
 int bootflow_iter_check_blk(const struct bootflow_iter *iter);
 
+/**
+ * bootflow_iter_check_mmc() - Check that a bootflow uses a MMC device
+ *
+ * This checks the bootdev in the bootflow to make sure it uses a mmc device
+ *
+ * Return: 0 if OK, -ENOTSUPP if some other device is used (e.g. ethernet)
+ */
+int bootflow_iter_check_mmc(const struct bootflow_iter *iter);
+
 /**
  * bootflow_iter_check_sf() - Check that a bootflow uses SPI FLASH
  *

-- 
2.45.2



[PATCH v2 1/5] boot: android: Provide vendor_bootimg_addr in boot_get_fdt()

2024-06-13 Thread Mattijs Korpershoek
When calling android_image_get_dtb_by_index() using boot image v3+,
we should also pass the vendor_boot ramdisk address.

Use get_avendor_bootimg_addr() to do so.

Notes: on boot image v2, this is harmless since get_avendor_bootimg_addr()
   returns -1.
   for legacy implementations that don't have CMD_ABOOTIMG, add a weak
   implementation to avoid linking errors.

Signed-off-by: Mattijs Korpershoek 
---
 boot/image-android.c | 5 +
 boot/image-fdt.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index ee626972c114..09c7a44e058a 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -56,6 +56,11 @@ static ulong add_trailer(ulong bootconfig_start_addr, ulong 
bootconfig_size)
return BOOTCONFIG_TRAILER_SIZE;
 }
 
+__weak ulong get_avendor_bootimg_addr(void)
+{
+   return -1;
+}
+
 static void android_boot_image_v3_v4_parse_hdr(const struct 
andr_boot_img_hdr_v3 *hdr,
   struct andr_image_data *data)
 {
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index 56dd7687f51c..8332792b8e80 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -502,7 +502,7 @@ int boot_get_fdt(void *buf, const char *select, uint arch,
 * Firstly check if this android boot image has dtb field.
 */
dtb_idx = (u32)env_get_ulong("adtb_idx", 10, 0);
-   if (android_image_get_dtb_by_index((ulong)hdr, 0,
+   if (android_image_get_dtb_by_index((ulong)hdr, 
get_avendor_bootimg_addr(),
   dtb_idx, _addr, 
_size)) {
fdt_blob = (char *)map_sysmem(fdt_addr, 0);
if (fdt_check_header(fdt_blob))

-- 
2.45.2



[PATCH v2 0/5] bootstd: Add Android support

2024-06-13 Thread Mattijs Korpershoek
Android boot flow is a bit different than a regular Linux distro.
Android relies on multiple partitions in order to boot.

A typical boot flow would be:
1. Parse the Bootloader Control Block (BCB, misc partition)
2. If BCB requested bootonce-bootloader, start fastboot and wait.
3. If BCB requested recovery or normal android, run the following:
   a. Get slot (A/B) from BCB
   b. Run AVB (Android Verified Boot) on boot partitions
   c. Load boot and vendor_boot partitions
   d. Load device-tree, ramdisk and boot

The AOSP documentation has more details at [1], [2], [3]

This has been implemented via complex boot scripts such as [4].
However, these boot script are neither very maintainable nor generic.
Moreover, DISTRO_DEFAULTS is being deprecated [5].

Add a generic Android bootflow implementation for bootstd.

For this initial version, only boot image v4 is supported.

This has been tested on sandbox using:
$ ./test/py/test.py --bd sandbox --build -k test_ut

This has also been tested on the AM62X SK EVM using TI's Android SDK[6]
To test on TI board, the following (WIP) patch is needed as well:
https://gitlab.baylibre.com/baylibre/ti/ti-u-boot/-/commit/84cceb912bccd7cdd7f9dd69bca0e5d987a1fd04

[1] https://source.android.com/docs/core/architecture/bootloader
[2] https://source.android.com/docs/core/architecture/partitions
[3] https://source.android.com/docs/core/architecture/partitions/generic-boot
[4] 
https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_android.h
[5] https://lore.kernel.org/r/all/20230914165615.1058529-17-...@chromium.org/
[6] 
https://software-dl.ti.com/processor-sdk-android/esd/AM62X/09_02_00/docs/android/Overview.html

Signed-off-by: Mattijs Korpershoek 
---
Changes in v2:
- Dropped patch 2/6 boot: android: Add image_android_get_version() (Igor)
- Fixed multi-line comment style (Igor, Simon)
- Added dependency on CMD_FASTBOOT for BOOTMETH_ANDROID (Igor)
- Fixed various resource leaks (Igor)
- Fixed bootmeth_priv dangling pointer on error cases (Igor)
- Updated test instructions in commit message for patch 6/6
- Added __weak impl of get_avendor_bootimg_addr() in patch 1 (dropped
  Igor's review because of this change)
- Added extra info in Kconfig to detail MMC limitation (Simon)
- Fixed typo Bootmethod->Bootmeth (Simon)
- Documented android_priv structure (Simon)
- Demoted various messages from printf() to log_debug (Simon)
- Fixed some lines too long (Simon)
- Added function documentation to read_slotted_partition() (Simon)
- Added some doc about avb extra_args being modified (Simon)
- Link to v1: 
https://lore.kernel.org/r/20240606-bootmeth-android-v1-0-0c69d4457...@baylibre.com

---
Mattijs Korpershoek (5):
  boot: android: Provide vendor_bootimg_addr in boot_get_fdt()
  bootstd: Add bootflow_iter_check_mmc() helper
  android: boot: Add set_abootimg_addr() and set_avendor_bootimg_addr()
  bootstd: Add a bootmeth for Android
  bootstd: Add test for bootmeth_android

 MAINTAINERS   |   7 +
 arch/sandbox/dts/test.dts |   8 +
 boot/Kconfig  |  16 ++
 boot/Makefile |   2 +
 boot/bootflow.c   |  12 +
 boot/bootmeth_android.c   | 553 ++
 boot/bootmeth_android.h   |  29 +++
 boot/image-android.c  |   5 +
 boot/image-fdt.c  |   2 +-
 cmd/abootimg.c|  10 +
 configs/sandbox_defconfig |   2 +-
 doc/develop/bootstd.rst   |   6 +
 include/bootflow.h|   9 +
 include/image.h   |  14 ++
 test/boot/bootflow.c  |  65 +-
 test/py/tests/test_ut.py  |  76 +++
 16 files changed, 811 insertions(+), 5 deletions(-)
---
base-commit: f9886bc60f42d5bcfcfa4e474af7dc230400b6be
change-id: 20240605-bootmeth-android-bfc8596e9367

Best regards,
-- 
Mattijs Korpershoek 



[PATCH v2 3/5] android: boot: Add set_abootimg_addr() and set_avendor_bootimg_addr()

2024-06-13 Thread Mattijs Korpershoek
The only way to configure the load addresses for both bootimg and
vendor_bootimg is by using the "abootimg" command.
If we want to use the C API, there is no equivalent.

Add set_abootimg_addr() and set_avendor_bootimg_addr() so that we can
specify the load address from C.

This can be useful for implementing an Android bootmethod.

Reviewed-by: Igor Opaniuk 
Signed-off-by: Mattijs Korpershoek 
---
 cmd/abootimg.c  | 10 ++
 include/image.h | 14 ++
 2 files changed, 24 insertions(+)

diff --git a/cmd/abootimg.c b/cmd/abootimg.c
index 327712a536c0..ae7a1a7c83b0 100644
--- a/cmd/abootimg.c
+++ b/cmd/abootimg.c
@@ -22,6 +22,11 @@ ulong get_abootimg_addr(void)
return (_abootimg_addr == -1 ? image_load_addr : _abootimg_addr);
 }
 
+void set_abootimg_addr(ulong addr)
+{
+   _abootimg_addr = addr;
+}
+
 ulong get_ainit_bootimg_addr(void)
 {
return _ainit_bootimg_addr;
@@ -32,6 +37,11 @@ ulong get_avendor_bootimg_addr(void)
return _avendor_bootimg_addr;
 }
 
+void set_avendor_bootimg_addr(ulong addr)
+{
+   _avendor_bootimg_addr = addr;
+}
+
 static int abootimg_get_ver(int argc, char *const argv[])
 {
const struct andr_boot_img_hdr_v0 *hdr;
diff --git a/include/image.h b/include/image.h
index c5b288f62b44..df2decbf5c2a 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1970,6 +1970,13 @@ bool is_android_vendor_boot_image_header(const void 
*vendor_boot_img);
  */
 ulong get_abootimg_addr(void);
 
+/**
+ * set_abootimg_addr() - Set Android boot image address
+ *
+ * Return: no returned results
+ */
+void set_abootimg_addr(ulong addr);
+
 /**
  * get_ainit_bootimg_addr() - Get Android init boot image address
  *
@@ -1984,6 +1991,13 @@ ulong get_ainit_bootimg_addr(void);
  */
 ulong get_avendor_bootimg_addr(void);
 
+/**
+ * set_abootimg_addr() - Set Android vendor boot image address
+ *
+ * Return: no returned results
+ */
+void set_avendor_bootimg_addr(ulong addr);
+
 /**
  * board_fit_config_name_match() - Check for a matching board name
  *

-- 
2.45.2



[PATCH v2 5/5] bootstd: Add test for bootmeth_android

2024-06-13 Thread Mattijs Korpershoek
Add a unit test for testing the Android bootmethod.

This requires another mmc image (mmc7) to contain the following partitions:
- misc: contains the Bootloader Control Block (BCB)
- boot_a: contains a fake generic kernel image
- vendor_boot_a: contains a fake vendor_boot image

Also add BOOTMETH_ANDROID as a dependency on sandbox so that we can test
this with:

$ ./test/py/test.py --bd sandbox --build -k test_abootimg # build bootv4.img
$ ./test/py/test.py --bd sandbox --build -k test_ut # build the mmc7.img
$ ./test/py/test.py --bd sandbox --build -k bootflow_android

Reviewed-by: Simon Glass 
Signed-off-by: Mattijs Korpershoek 
---
 arch/sandbox/dts/test.dts |  8 +
 configs/sandbox_defconfig |  2 +-
 test/boot/bootflow.c  | 65 ++--
 test/py/tests/test_ut.py  | 76 +++
 4 files changed, 147 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index a012f5c4c9ba..5fb5eac862ec 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -43,6 +43,7 @@
mmc4 = "/mmc4";
mmc5 = "/mmc5";
mmc6 = "/mmc6";
+   mmc7 = "/mmc7";
pci0 = 
pci1 = 
pci2 = 
@@ -1129,6 +1130,13 @@
filename = "mmc6.img";
};
 
+   /* This is used for Android tests */
+   mmc7 {
+   status = "disabled";
+   compatible = "sandbox,mmc";
+   filename = "mmc7.img";
+   };
+
pch {
compatible = "sandbox,pch";
};
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 93b52f2de5cf..bc4398f101a7 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -15,6 +15,7 @@ CONFIG_FIT=y
 CONFIG_FIT_RSASSA_PSS=y
 CONFIG_FIT_CIPHER=y
 CONFIG_FIT_VERBOSE=y
+CONFIG_BOOTMETH_ANDROID=y
 CONFIG_LEGACY_IMAGE_FORMAT=y
 CONFIG_MEASURED_BOOT=y
 CONFIG_BOOTSTAGE=y
@@ -40,7 +41,6 @@ CONFIG_LOG_MAX_LEVEL=9
 CONFIG_LOG_DEFAULT_LEVEL=6
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_STACKPROTECTOR=y
-CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_SMBIOS=y
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 4511cfa7f9bf..934c4dcbad2b 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -27,6 +27,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+extern U_BOOT_DRIVER(bootmeth_android);
 extern U_BOOT_DRIVER(bootmeth_cros);
 extern U_BOOT_DRIVER(bootmeth_2script);
 
@@ -518,12 +519,12 @@ BOOTSTD_TEST(bootflow_cmd_boot, UT_TESTF_DM | 
UT_TESTF_SCAN_FDT);
  * @uts: Unit test state
  * @mmc_dev: MMC device to use, e.g. "mmc4". Note that this must remain valid
  * in the caller until
- * @bind_cros: true to bind the ChromiumOS bootmeth
+ * @bind_cros: true to bind the ChromiumOS and Android bootmeths
  * @old_orderp: Returns the original bootdev order, which must be restored
  * Returns 0 on success, -ve on failure
  */
 static int prep_mmc_bootdev(struct unit_test_state *uts, const char *mmc_dev,
-   bool bind_cros, const char ***old_orderp)
+   bool bind_cros_android, const char ***old_orderp)
 {
static const char *order[] = {"mmc2", "mmc1", NULL, NULL};
struct udevice *dev, *bootstd;
@@ -545,12 +546,19 @@ static int prep_mmc_bootdev(struct unit_test_state *uts, 
const char *mmc_dev,
"bootmeth_script", 0, ofnode_null(), ));
 
/* Enable the cros bootmeth if needed */
-   if (IS_ENABLED(CONFIG_BOOTMETH_CROS) && bind_cros) {
+   if (IS_ENABLED(CONFIG_BOOTMETH_CROS) && bind_cros_android) {
ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, ));
ut_assertok(device_bind(bootstd, DM_DRIVER_REF(bootmeth_cros),
"cros", 0, ofnode_null(), ));
}
 
+   /* Enable the android bootmeths if needed */
+   if (IS_ENABLED(CONFIG_BOOTMETH_ANDROID) && bind_cros_android) {
+   ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, ));
+   ut_assertok(device_bind(bootstd, 
DM_DRIVER_REF(bootmeth_android),
+   "android", 0, ofnode_null(), ));
+   }
+
/* Change the order to include the device */
std = dev_get_priv(bootstd);
old_order = std->bootdev_order;
@@ -589,6 +597,37 @@ static int scan_mmc_bootdev(struct unit_test_state *uts, 
const char *mmc_dev,
return 0;
 }
 
+/**
+ * scan_mmc_android_bootdev() - Set up an mmc bootdev so we can access other
+ * distros. Android bootflow might print "ANDROID:*" while scanning
+ *
+ * @uts: Unit test state
+ * @mmc_dev: MMC device to use, e.g. "mmc4"
+ * Returns 0 on suc

[PATCH v2 4/5] bootstd: Add a bootmeth for Android

2024-06-13 Thread Mattijs Korpershoek
Android boot flow is a bit different than a regular Linux distro.
Android relies on multiple partitions in order to boot.

A typical boot flow would be:
1. Parse the Bootloader Control Block (BCB, misc partition)
2. If BCB requested bootonce-bootloader, start fastboot and wait.
3. If BCB requested recovery or normal android, run the following:
3.a. Get slot (A/B) from BCB
3.b. Run AVB (Android Verified Boot) on boot partitions
3.c. Load boot and vendor_boot partitions
3.d. Load device-tree, ramdisk and boot

The AOSP documentation has more details at [1], [2], [3]

This has been implemented via complex boot scripts such as [4].
However, these boot script are neither very maintainable nor generic.
Moreover, DISTRO_DEFAULTS is being deprecated [5].

Add a generic Android bootflow implementation for bootstd.
For this initial version, only boot image v4 is supported.

[1] https://source.android.com/docs/core/architecture/bootloader
[2] https://source.android.com/docs/core/architecture/partitions
[3] https://source.android.com/docs/core/architecture/partitions/generic-boot
[4] 
https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_android.h
[5] https://lore.kernel.org/r/all/20230914165615.1058529-17-...@chromium.org/

Reviewed-by: Simon Glass 
Signed-off-by: Mattijs Korpershoek 
---
 MAINTAINERS |   7 +
 boot/Kconfig|  16 ++
 boot/Makefile   |   2 +
 boot/bootmeth_android.c | 553 
 boot/bootmeth_android.h |  29 +++
 doc/develop/bootstd.rst |   6 +
 6 files changed, 613 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 66783d636e3d..6d2b87720565 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -939,6 +939,13 @@ F: include/bootstd.h
 F: net/eth_bootdevice.c
 F: test/boot/
 
+BOOTMETH_ANDROID
+M: Mattijs Korpershoek 
+S: Maintained
+T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git
+F: boot/bootmeth_android.c
+F: boot/bootmeth_android.h
+
 BTRFS
 M: Marek Behún 
 R: Qu Wenruo 
diff --git a/boot/Kconfig b/boot/Kconfig
index 6f3096c15a6f..88266c8d2ed3 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -494,6 +494,22 @@ config BOOTMETH_GLOBAL
  EFI bootmgr, since they take full control over which bootdevs are
  selected to boot.
 
+config BOOTMETH_ANDROID
+   bool "Bootdev support for Android"
+   depends on X86 || ARM || SANDBOX
+   select ANDROID_AB
+   select ANDROID_BOOT_IMAGE
+   select CMD_BCB
+   select CMD_FASTBOOT
+   select PARTITION_TYPE_GUID
+   select PARTITION_UUIDS
+   help
+ Enables support for booting Android using bootstd. Android requires
+ multiple partitions (misc, boot, vbmeta, ...) in storage for booting.
+
+ Note that only MMC bootdevs are supported at present. This is caused
+ by AVB being limited to MMC devices only.
+
 config BOOTMETH_CROS
bool "Bootdev support for Chromium OS"
depends on X86 || ARM || SANDBOX
diff --git a/boot/Makefile b/boot/Makefile
index 84ccfeaecec4..75d1cd46fabf 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -66,3 +66,5 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_REQUEST) += vbe_request.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_OS) += vbe_simple_os.o
+
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_ANDROID) += bootmeth_android.o
diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
new file mode 100644
index ..6e8d3e615db0
--- /dev/null
+++ b/boot/bootmeth_android.c
@@ -0,0 +1,553 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Bootmeth for Android
+ *
+ * Copyright (C) 2024 BayLibre, SAS
+ * Written by Mattijs Korpershoek 
+ */
+#define LOG_CATEGORY UCLASS_BOOTSTD
+
+#include 
+#include 
+#if CONFIG_IS_ENABLED(AVB_VERIFY)
+#include 
+#endif
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "bootmeth_android.h"
+
+#define BCB_FIELD_COMMAND_SZ 32
+#define BCB_PART_NAME "misc"
+#define BOOT_PART_NAME "boot"
+#define VENDOR_BOOT_PART_NAME "vendor_boot"
+
+/**
+ * struct android_priv - Private data
+ *
+ * This is read from the disk and recorded for use when the full Android
+ * kernel must be loaded and booted
+ *
+ * @boot_mode: Requested boot mode (normal, recovery, bootloader)
+ * @slot: Nul-terminated partition slot suffix read from BCB ("a\0" or "b\0")
+ * @header_version: Android boot image header version
+ */
+struct android_priv {
+   enum android_boot_mode boot_mode;
+   char slot[2];
+   u32 header_version;
+};
+
+static int android_check(struct udevice *dev, struct bootflow_iter *iter)
+{
+   /* This only works on mmc devices */
+   if (bootflow_iter_check_mmc(iter))
+   return log_msg

Re: [PATCH 5/6] bootstd: Add a bootmeth for Android

2024-06-12 Thread Mattijs Korpershoek
Hi Simon,

Thank you for your review.

On mar., juin 11, 2024 at 12:52, Simon Glass  wrote:

> Hi Mattijs,
>
> On Thu, 6 Jun 2024 at 06:24, Mattijs Korpershoek
>  wrote:
>>
>> Android boot flow is a bit different than a regular Linux distro.
>> Android relies on multiple partitions in order to boot.
>>
>> A typical boot flow would be:
>> 1. Parse the Bootloader Control Block (BCB, misc partition)
>> 2. If BCB requested bootonce-bootloader, start fastboot and wait.
>> 3. If BCB requested recovery or normal android, run the following:
>> 3.a. Get slot (A/B) from BCB
>> 3.b. Run AVB (Android Verified Boot) on boot partitions
>> 3.c. Load boot and vendor_boot partitions
>> 3.d. Load device-tree, ramdisk and boot
>>
>> The AOSP documentation has more details at [1], [2], [3]
>>
>> This has been implemented via complex boot scripts such as [4].
>> However, these boot script are neither very maintainable nor generic.
>> Moreover, DISTRO_DEFAULTS is being deprecated [5].
>>
>> Add a generic Android bootflow implementation for bootstd.
>> For this initial version, only boot image v4 is supported.
>>
>> [1] https://source.android.com/docs/core/architecture/bootloader
>> [2] https://source.android.com/docs/core/architecture/partitions
>> [3] https://source.android.com/docs/core/architecture/partitions/generic-boot
>> [4] 
>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_android.h
>> [5] https://lore.kernel.org/r/all/20230914165615.1058529-17-...@chromium.org/
>>
>> Signed-off-by: Mattijs Korpershoek 
>> ---
>>  MAINTAINERS |   7 +
>>  boot/Kconfig|  14 ++
>>  boot/Makefile   |   2 +
>>  boot/bootmeth_android.c | 522 
>> 
>>  boot/bootmeth_android.h |  27 +++
>>  doc/develop/bootstd.rst |   6 +
>>  6 files changed, 578 insertions(+)
>
> Reviewed-by: Simon Glass 
>
> nits below
>
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 66783d636e3d..6d2b87720565 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -939,6 +939,13 @@ F: include/bootstd.h
>>  F: net/eth_bootdevice.c
>>  F: test/boot/
>>
>> +BOOTMETH_ANDROID
>> +M: Mattijs Korpershoek 
>> +S: Maintained
>> +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git
>> +F: boot/bootmeth_android.c
>> +F: boot/bootmeth_android.h
>> +
>>  BTRFS
>>  M: Marek Behún 
>>  R: Qu Wenruo 
>> diff --git a/boot/Kconfig b/boot/Kconfig
>> index 6f3096c15a6f..5fa6f3b8315d 100644
>> --- a/boot/Kconfig
>> +++ b/boot/Kconfig
>> @@ -494,6 +494,20 @@ config BOOTMETH_GLOBAL
>>   EFI bootmgr, since they take full control over which bootdevs are
>>   selected to boot.
>>
>> +config BOOTMETH_ANDROID
>> +   bool "Bootdev support for Android"
>> +   depends on X86 || ARM || SANDBOX
>> +   select ANDROID_AB
>> +   select ANDROID_BOOT_IMAGE
>> +   select CMD_BCB
>> +   select PARTITION_TYPE_GUID
>> +   select PARTITION_UUIDS
>> +   help
>> + Enables support for booting Android using bootdevs. Android 
>> requires
>
> using standard boot (or using bootstd).

Will do for v2.

>
>> + multiple partitions (misc, boot, vbmeta, ...) in storage for 
>> booting.
>> +
>> + Note that only MMC bootdevs are supported at present.
>
> Why is that limitation present? Can you please mention what is needed
> to remove it?

Mainly because we use AVB and AVB is hard-coded for MMC. Alistair
submitted changes to convert to generic block devices here:

https://lore.kernel.org/all/20220926220211.868968-1-ade...@google.com/

There were some review comments but I did not see any v2 on the list.

I will add a comment in the KConfig description.

>
>> +
>>  config BOOTMETH_CROS
>> bool "Bootdev support for Chromium OS"
>> depends on X86 || ARM || SANDBOX
>> diff --git a/boot/Makefile b/boot/Makefile
>> index 84ccfeaecec4..75d1cd46fabf 100644
>> --- a/boot/Makefile
>> +++ b/boot/Makefile
>> @@ -66,3 +66,5 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_REQUEST) += 
>> vbe_request.o
>>  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
>>  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o
>>  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_OS) += vbe_simple_os.o
>> +
>> +obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_ANDROID) += bootmeth_and

Re: [PATCH 5/6] bootstd: Add a bootmeth for Android

2024-06-11 Thread Mattijs Korpershoek
Hi Igor,

Thank you for your quick review.

On lun., juin 10, 2024 at 17:15, Igor Opaniuk  wrote:

> Hi Mattijs,
>
> On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek
>  wrote:
>>
>> Android boot flow is a bit different than a regular Linux distro.
>> Android relies on multiple partitions in order to boot.
>>
>> A typical boot flow would be:
>> 1. Parse the Bootloader Control Block (BCB, misc partition)
>> 2. If BCB requested bootonce-bootloader, start fastboot and wait.
>> 3. If BCB requested recovery or normal android, run the following:
>> 3.a. Get slot (A/B) from BCB
>> 3.b. Run AVB (Android Verified Boot) on boot partitions
>> 3.c. Load boot and vendor_boot partitions
>> 3.d. Load device-tree, ramdisk and boot
>>
>> The AOSP documentation has more details at [1], [2], [3]
>>
>> This has been implemented via complex boot scripts such as [4].
>> However, these boot script are neither very maintainable nor generic.
>> Moreover, DISTRO_DEFAULTS is being deprecated [5].
>>
>> Add a generic Android bootflow implementation for bootstd.
>> For this initial version, only boot image v4 is supported.
>>
>> [1] https://source.android.com/docs/core/architecture/bootloader
>> [2] https://source.android.com/docs/core/architecture/partitions
>> [3] https://source.android.com/docs/core/architecture/partitions/generic-boot
>> [4] 
>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_android.h
>> [5] https://lore.kernel.org/r/all/20230914165615.1058529-17-...@chromium.org/
>>
>> Signed-off-by: Mattijs Korpershoek 
>> ---
>>  MAINTAINERS |   7 +
>>  boot/Kconfig|  14 ++
>>  boot/Makefile   |   2 +
>>  boot/bootmeth_android.c | 522 
>> 
>>  boot/bootmeth_android.h |  27 +++
>>  doc/develop/bootstd.rst |   6 +
>>  6 files changed, 578 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 66783d636e3d..6d2b87720565 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -939,6 +939,13 @@ F: include/bootstd.h
>>  F: net/eth_bootdevice.c
>>  F: test/boot/
>>
>> +BOOTMETH_ANDROID
>> +M: Mattijs Korpershoek 
>> +S: Maintained
>> +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git
>> +F: boot/bootmeth_android.c
>> +F: boot/bootmeth_android.h
>> +
>>  BTRFS
>>  M: Marek Behún 
>>  R: Qu Wenruo 
>> diff --git a/boot/Kconfig b/boot/Kconfig
>> index 6f3096c15a6f..5fa6f3b8315d 100644
>> --- a/boot/Kconfig
>> +++ b/boot/Kconfig
>> @@ -494,6 +494,20 @@ config BOOTMETH_GLOBAL
>>   EFI bootmgr, since they take full control over which bootdevs are
>>   selected to boot.
>>
>> +config BOOTMETH_ANDROID
>> +   bool "Bootdev support for Android"
>> +   depends on X86 || ARM || SANDBOX
>> +   select ANDROID_AB
>> +   select ANDROID_BOOT_IMAGE
>> +   select CMD_BCB
>> +   select PARTITION_TYPE_GUID
>> +   select PARTITION_UUIDS
>> +   help
>> + Enables support for booting Android using bootdevs. Android 
>> requires
>> + multiple partitions (misc, boot, vbmeta, ...) in storage for 
>> booting.
>> +
>> + Note that only MMC bootdevs are supported at present.
>> +
>>  config BOOTMETH_CROS
>> bool "Bootdev support for Chromium OS"
>> depends on X86 || ARM || SANDBOX
>> diff --git a/boot/Makefile b/boot/Makefile
>> index 84ccfeaecec4..75d1cd46fabf 100644
>> --- a/boot/Makefile
>> +++ b/boot/Makefile
>> @@ -66,3 +66,5 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_REQUEST) += 
>> vbe_request.o
>>  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
>>  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o
>>  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_OS) += vbe_simple_os.o
>> +
>> +obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_ANDROID) += bootmeth_android.o
>> diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
>> new file mode 100644
>> index ..26d548d2fd6e
>> --- /dev/null
>> +++ b/boot/bootmeth_android.c
>> @@ -0,0 +1,522 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Bootmethod for Android
>> + *
>> + * Copyright (C) 2024 BayLibre, SAS
>> + * Written by Mattijs Korpershoek 
>> + */
>> +#define LOG_CATEGORY UCLASS_BOOTSTD
>> +
>> +#include 
>> +#include 
>

Re: [PATCH 3/6] usb: gadget: Drop usb_gadget_controller_number()

2024-06-11 Thread Mattijs Korpershoek
Hi Lukasz,

On mar., juin 11, 2024 at 10:51, Lukasz Majewski  wrote:

> On Tue, 11 Jun 2024 09:20:33 +0200
> Mattijs Korpershoek  wrote:

[...]

>
>> > -- 
>> > 2.43.0  
>
> FInally. :-)
>
> Thanks Mattijs for this cleanup.

You should thank Marek, i've just reviewed his work :)

>
> Reviewed-by: Lukasz Majewski 
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH,  Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de


Re: [PATCH 3/6] bootstd: Add bootflow_iter_check_mmc() helper

2024-06-11 Thread Mattijs Korpershoek
Hi Igor,

Thank you for the review.

On lun., juin 10, 2024 at 11:31, Igor Opaniuk  wrote:

> Hi Mattijs,
>
> On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek
>  wrote:
>>
>> Some bootflows might be able to only boot from MMC devices.
>>
>> Add a helper function these bootflows can use.
>>
>> Signed-off-by: Mattijs Korpershoek 
>> ---
>>  boot/bootflow.c| 12 
>>  include/bootflow.h |  9 +
>>  2 files changed, 21 insertions(+)
>>
>> diff --git a/boot/bootflow.c b/boot/bootflow.c
>> index 9aa3179c3881..59d77d2385f4 100644
>> --- a/boot/bootflow.c
>> +++ b/boot/bootflow.c
>> @@ -575,6 +575,18 @@ int bootflow_iter_check_blk(const struct bootflow_iter 
>> *iter)
>> return -ENOTSUPP;
>>  }
>>
>> +int bootflow_iter_check_mmc(const struct bootflow_iter *iter)
>> +{
>> +   const struct udevice *media = dev_get_parent(iter->dev);
>> +   enum uclass_id id = device_get_uclass_id(media);
>> +
>> +   log_debug("uclass %d: %s\n", id, uclass_get_name(id));
>> +   if (id == UCLASS_MMC)
>> +   return 0;
>> +
>> +   return -ENOTSUPP;
>> +}
>> +
>>  int bootflow_iter_check_sf(const struct bootflow_iter *iter)
>>  {
>> const struct udevice *media = dev_get_parent(iter->dev);
>> diff --git a/include/bootflow.h b/include/bootflow.h
>> index 080ee8501225..6058ddd89b16 100644
>> --- a/include/bootflow.h
>> +++ b/include/bootflow.h
>> @@ -407,6 +407,15 @@ void bootflow_remove(struct bootflow *bflow);
>>   */
>>  int bootflow_iter_check_blk(const struct bootflow_iter *iter);
>>
>> +/**
>> + * bootflow_iter_check_mmc() - Check that a bootflow uses a MMC device
>> + *
>> + * This checks the bootdev in the bootflow to make sure it uses a mmc device
>> + *
>> + * Return: 0 if OK, -ENOTSUPP if some other device is used (e.g. ethernet)
>> + */
>> +int bootflow_iter_check_mmc(const struct bootflow_iter *iter);
>> +
>>  /**
>>   * bootflow_iter_check_sf() - Check that a bootflow uses SPI FLASH
>>   *
>>
>> --
>> 2.45.0
>>
>
> Reviewed-by: Igor Opaniuk 
>
> A bit offtopic (just an idea for future refactoring), but I think all these
> bootflow_iter_check_* helpers should be replaced by just one
> int bootflow_iter_check_id(const struct bootflow_iter *iter, enum uclass_id 
> id)
> to avoid code duplication or at least keep all these
> bootmedia-specific functions as
> wrappers with one-line call to bootflow_iter_check_id(iter,
> UCLASS_SPI_*) inside.

I like this idea as well, I'll consider to implement this as a future 
refactoring.

>
> -- 
> Best regards - Atentamente - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opan...@gmail.com
> skype: igor.opanyuk
> https://www.linkedin.com/in/iopaniuk


Re: [PATCH 2/6] boot: android: Add image_android_get_version()

2024-06-11 Thread Mattijs Korpershoek
Hi Igor,

Thank you for the review.

On lun., juin 10, 2024 at 11:20, Igor Opaniuk  wrote:

> Hi Mattijs,
>
> On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek
>  wrote:
>>
>> When reading a boot image header, we may need to retrieve the header
>> version.
>>
>> Add a helper function for it.
>>
>> Signed-off-by: Mattijs Korpershoek 
>> ---
>>  boot/image-android.c | 7 ++-
>>  include/image.h  | 7 +++
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/boot/image-android.c b/boot/image-android.c
>> index ddd8ffd5e540..4f8fb51585eb 100644
>> --- a/boot/image-android.c
>> +++ b/boot/image-android.c
>> @@ -185,7 +185,7 @@ bool android_image_get_data(const void *boot_hdr, const 
>> void *vendor_boot_hdr,
>> return false;
>> }
>>
>> -   if (((struct andr_boot_img_hdr_v0 *)boot_hdr)->header_version > 2) {
>> +   if (android_image_get_version(boot_hdr) > 2) {
>> if (!vendor_boot_hdr) {
>> printf("For boot header v3+ vendor boot image has to 
>> be provided\n");
>> return false;
>> @@ -203,6 +203,11 @@ bool android_image_get_data(const void *boot_hdr, const 
>> void *vendor_boot_hdr,
>> return true;
>>  }
>>
>> +u32 android_image_get_version(const void *hdr)
>> +{
>> +   return ((struct andr_boot_img_hdr_v0 *)hdr)->header_version;
>> +}
>> +
>>  static ulong android_image_get_kernel_addr(struct andr_image_data *img_data)
>>  {
>> /*
>> diff --git a/include/image.h b/include/image.h
>> index acffd17e0dfd..18e5ced5ab42 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -1963,6 +1963,13 @@ bool is_android_boot_image_header(const void *hdr);
>>   */
>>  bool is_android_vendor_boot_image_header(const void *vendor_boot_img);
>>
>> +/**
>> + * android_image_get_version() - Retrieve the boot.img version
>> + *
>> + * Return: Android boot image header version.
>> + */
>> +u32 android_image_get_version(const void *hdr);
>> +
>>  /**
>>   * get_abootimg_addr() - Get Android boot image address
>>   *
>>
>> --
>> 2.45.0
>>
> why introduce a helper function if there is only one user of it?

I added a second user of the helper function in patch 5/6.

>
> android_image_get_data() expects andr_boot_img_hdr_v0 anyway,
> as it has an explicit check for it in the very beginning
> (is_android_boot_image_header()).

Right.

>
> Have you considered adjusting android_image_get_data() declaration, and just 
> use
> andr_boot_img_hdr_v0 *boot_hdr as first param instead (like it's done
> for example in
> android_boot_image_v0_v1_v2_parse_hdr()) and then rely on implicit
> cast when this
> function is used.
>
> this is of course all a matter of preference, just thinking out loud

I've given this some more thought, and since I'm already using
struct andr_boot_img_hdr_v0 in bootmeth_android/scan_boot_part(), I will
drop this patch for v2.

The helper seems indeed a bit useless given that we can use the struct's
member for this.

Thanks!

>
> -- 
> Best regards - Atentamente - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opan...@gmail.com
> skype: igor.opanyuk
> https://www.linkedin.com/in/iopaniuk


Re: [PATCH 6/6] usb: dwc3: gadget: Convert epautoconf workaround to match_ep callback

2024-06-11 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On dim., juin 09, 2024 at 23:32, Marek Vasut  
wrote:

> Use the .match_ep() callback instead of workaround in core code.
> Replace descriptor parsing with ch9 macros with the same effect.
> Drop the SPL specific behavior, it is unclear why SPL should even
> be special.

Li, Peng,

Is this good for you as well?

You seem to be the author/committer of:
c93edbf5385e ("usb: gadget: don't change ep name for dwc3 while ep autoconfig")

I'd like to make sure this patch does not break your use-case.
Please let me know within 2 weeks or so, otherwise I'll apply the changes.

>
> Signed-off-by: Marek Vasut 

To me, this looks good.

Reviewed-by: Mattijs Korpershoek 
Tested-by: Mattijs Korpershoek  # on vim3

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/dwc3/gadget.c   | 33 +++
>  drivers/usb/gadget/epautoconf.c | 46 +
>  2 files changed, 34 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index fab32575647..3ef2f016a60 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1606,6 +1606,38 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>   return 0;
>  }
>  
> +static struct usb_ep *dwc3_find_ep(struct usb_gadget *gadget, const char 
> *name)
> +{
> + struct usb_ep *ep;
> +
> + list_for_each_entry(ep, >ep_list, ep_list)
> + if (!strcmp(ep->name, name))
> + return ep;
> +
> + return NULL;
> +}
> +
> +static struct
> +usb_ep *dwc3_gadget_match_ep(struct usb_gadget *gadget,
> +  struct usb_endpoint_descriptor *desc,
> +  struct usb_ss_ep_comp_descriptor *comp_desc)
> +{
> + /*
> +  * First try standard, common configuration: ep1in-bulk,
> +  * ep2out-bulk, ep3in-int to match other udc drivers to avoid
> +  * confusion in already deployed software (endpoint numbers
> +  * hardcoded in userspace software/drivers)
> +  */
> + if (usb_endpoint_is_bulk_in(desc))
> + return dwc3_find_ep(gadget, "ep1in");
> + if (usb_endpoint_is_bulk_out(desc))
> + return dwc3_find_ep(gadget, "ep2out");
> + if (usb_endpoint_is_int_in(desc))
> + return dwc3_find_ep(gadget, "ep3in");
> +
> + return NULL;
> +}
> +
>  static const struct usb_gadget_ops dwc3_gadget_ops = {
>   .get_frame  = dwc3_gadget_get_frame,
>   .wakeup = dwc3_gadget_wakeup,
> @@ -1613,6 +1645,7 @@ static const struct usb_gadget_ops dwc3_gadget_ops = {
>   .pullup = dwc3_gadget_pullup,
>   .udc_start  = dwc3_gadget_start,
>   .udc_stop   = dwc3_gadget_stop,
> + .match_ep   = dwc3_gadget_match_ep,
>  };
>  
>  /* 
> -- */
> diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
> index 66599ce8efa..a4da4f72de9 100644
> --- a/drivers/usb/gadget/epautoconf.c
> +++ b/drivers/usb/gadget/epautoconf.c
> @@ -166,18 +166,6 @@ static int ep_matches(
>   return 1;
>  }
>  
> -static struct usb_ep *
> -find_ep(struct usb_gadget *gadget, const char *name)
> -{
> - struct usb_ep   *ep;
> -
> - list_for_each_entry(ep, >ep_list, ep_list) {
> - if (0 == strcmp(ep->name, name))
> - return ep;
> - }
> - return NULL;
> -}
> -
>  /**
>   * usb_ep_autoconfig - choose an endpoint matching the descriptor
>   * @gadget: The device to which the endpoint must belong.
> @@ -213,39 +201,7 @@ struct usb_ep *usb_ep_autoconfig(
>   struct usb_endpoint_descriptor  *desc
>  )
>  {
> - struct usb_ep   *ep = NULL;
> - u8  type;
> -
> - type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> -
> - /* First, apply chip-specific "best usage" knowledge.
> -  * This might make a good usb_gadget_ops hook ...
> -  */
> - if (!IS_ENABLED(CONFIG_SPL_BUILD) &&
> - IS_ENABLED(CONFIG_USB_DWC3_GADGET) &&
> - !strcmp("dwc3-gadget", gadget->name)) {
> - const char *name = NULL;
> - /*
> -  * First try standard, common configuration: ep1in-bulk,
> -  * ep2out-bulk, ep3in-int to match other udc driver

Re: [PATCH 5/6] usb: gadget: Add full ep_matches() check past .match_ep() callback

2024-06-11 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On dim., juin 09, 2024 at 23:32, Marek Vasut  
wrote:

> If .match_ep() callback returns non-NULL endpoint, immediately check
> its usability and if the returned endpoint is usable, stop search and
> return the endpoint. Otherwise, continue with best effort search for
> usable endpoint.
>
> Currently the code would attempt the best effort search in any case,
> which may find another unexpected endpoint. It is likely that the
> intention of the original code was to stop the search early.
>
> Fixes: 77dcbdf3c1ce ("usb: gadget: Add match_ep() op to usb_gadget_ops")
> Signed-off-by: Marek Vasut 

I've added Vignesh to the CC list since he is the author of
77dcbdf3c1ce. He might be able to comment if this was indeed a mistake.

It looks like a good fix to me as well. With this change we match more closely
the linux implementation (usb_ep_autoconfig_ss()).

Reviewed-by: Mattijs Korpershoek 
Tested-by: Mattijs Korpershoek  # on vim3

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/epautoconf.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
> index 09950ceeaed..66599ce8efa 100644
> --- a/drivers/usb/gadget/epautoconf.c
> +++ b/drivers/usb/gadget/epautoconf.c
> @@ -247,8 +247,11 @@ struct usb_ep *usb_ep_autoconfig(
>   return ep;
>   }
>  
> - if (gadget->ops->match_ep)
> + if (gadget->ops->match_ep) {
>   ep = gadget->ops->match_ep(gadget, desc, NULL);
> + if (ep && ep_matches(gadget, ep, desc))
> + return ep;
> + }
>  
>   /* Second, look at endpoints until an unclaimed one looks usable */
>   list_for_each_entry(ep, >ep_list, ep_list) {
> -- 
> 2.43.0


Re: [PATCH 4/6] usb: gadget: Drop all gadget_is_*() functions

2024-06-11 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On dim., juin 09, 2024 at 23:32, Marek Vasut  
wrote:

> The only actually used gadget_is_*() functions are the one for DWC3
> used in epautoconf.c usb_ep_autoconfig() and one for MUSB in ether.c.
> The DWC3 one should be fixed in some separate patch.
>
> Inline the gadget_is_dwc3() and stop using ifdefs in favor of
> IS_ENABLED() macro.
>
> The rest of gadget_is_*() calls in usb_ep_autoconfig() can never
> be anything but 0, since those gadgets are not supported in U-Boot,
> so remove all that unused code. Remove gadget_chips.h as well.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 
Tested-by: Mattijs Korpershoek  # on vim3

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/epautoconf.c   |  40 +---
>  drivers/usb/gadget/ether.c|   8 +-
>  drivers/usb/gadget/gadget_chips.h | 148 --
>  3 files changed, 6 insertions(+), 190 deletions(-)
>  delete mode 100644 drivers/usb/gadget/gadget_chips.h
>
> diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
> index 0a70035ce04..09950ceeaed 100644
> --- a/drivers/usb/gadget/epautoconf.c
> +++ b/drivers/usb/gadget/epautoconf.c
> @@ -12,7 +12,6 @@
>  #include 
>  #include 
>  #include 
> -#include "gadget_chips.h"
>  
>  #define isdigit(c)  ('0' <= (c) && (c) <= '9')
>  
> @@ -222,41 +221,9 @@ struct usb_ep *usb_ep_autoconfig(
>   /* First, apply chip-specific "best usage" knowledge.
>* This might make a good usb_gadget_ops hook ...
>*/
> - if (gadget_is_net2280(gadget) && type == USB_ENDPOINT_XFER_INT) {
> - /* ep-e, ep-f are PIO with only 64 byte fifos */
> - ep = find_ep(gadget, "ep-e");
> - if (ep && ep_matches(gadget, ep, desc))
> - return ep;
> - ep = find_ep(gadget, "ep-f");
> - if (ep && ep_matches(gadget, ep, desc))
> - return ep;
> -
> - } else if (gadget_is_goku(gadget)) {
> - if (USB_ENDPOINT_XFER_INT == type) {
> - /* single buffering is enough */
> - ep = find_ep(gadget, "ep3-bulk");
> - if (ep && ep_matches(gadget, ep, desc))
> - return ep;
> - } else if (USB_ENDPOINT_XFER_BULK == type
> - && (USB_DIR_IN & desc->bEndpointAddress)) {
> - /* DMA may be available */
> - ep = find_ep(gadget, "ep2-bulk");
> - if (ep && ep_matches(gadget, ep, desc))
> - return ep;
> - }
> -
> - } else if (gadget_is_sh(gadget) && USB_ENDPOINT_XFER_INT == type) {
> - /* single buffering is enough; maybe 8 byte fifo is too */
> - ep = find_ep(gadget, "ep3in-bulk");
> - if (ep && ep_matches(gadget, ep, desc))
> - return ep;
> -
> - } else if (gadget_is_mq11xx(gadget) && USB_ENDPOINT_XFER_INT == type) {
> - ep = find_ep(gadget, "ep1-bulk");
> - if (ep && ep_matches(gadget, ep, desc))
> - return ep;
> -#ifndef CONFIG_SPL_BUILD
> - } else if (gadget_is_dwc3(gadget)) {
> + if (!IS_ENABLED(CONFIG_SPL_BUILD) &&
> + IS_ENABLED(CONFIG_USB_DWC3_GADGET) &&
> + !strcmp("dwc3-gadget", gadget->name)) {
>   const char *name = NULL;
>   /*
>* First try standard, common configuration: ep1in-bulk,
> @@ -278,7 +245,6 @@ struct usb_ep *usb_ep_autoconfig(
>   ep = find_ep(gadget, name);
>   if (ep && ep_matches(gadget, ep, desc))
>   return ep;
> -#endif
>   }
>  
>   if (gadget->ops->match_ep)
> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> index e76464e121b..b7b7bacb00d 100644
> --- a/drivers/usb/gadget/ether.c
> +++ b/drivers/usb/gadget/ether.c
> @@ -1989,13 +1989,11 @@ static int eth_bind(struct usb_gadget *gadget)
>* standard protocol is _strongly_ preferred for interop purposes.
>* (By everyone except Microsoft.)
>*/
> - if (gadget_is_musbhdrc(gadget)) {
> +
> + if (IS_ENABLED(CONFIG_USB_MUSB_GADGET) &&am

Re: [PATCH 3/6] usb: gadget: Drop usb_gadget_controller_number()

2024-06-11 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On dim., juin 09, 2024 at 23:32, Marek Vasut  
wrote:

> The bcdDevice field is defined as
> |Device release number in binary-coded decimal
> in the USB 2.0 specification. We use this field to distinguish the UDCs
> from each other. In theory this could be used on the host side to apply
> certain quirks if the "special" UDC in combination with this gadget is
> used. This hasn't been done as far as I am aware. In practice it would
> be better to fix the UDC driver before shipping since a later release
> might not need this quirk anymore.
>
> This patch removes the newly unused function. Linux stopped using this
> functionality in 2012, remove it from U-Boot as well.
>
> Matching Linux kernel commit:
> ed9cbda63d45 ("usb: gadget: remove usb_gadget_controller_number()")
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 
Tested-by: Mattijs Korpershoek  # on vim3

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/gadget_chips.h | 62 ---
>  1 file changed, 62 deletions(-)
>
> diff --git a/drivers/usb/gadget/gadget_chips.h 
> b/drivers/usb/gadget/gadget_chips.h
> index 98156c312d2..316051686c4 100644
> --- a/drivers/usb/gadget/gadget_chips.h
> +++ b/drivers/usb/gadget/gadget_chips.h
> @@ -146,65 +146,3 @@
>  #else
>  #define gadget_is_dwc2(g)0
>  #endif
> -
> -/**
> - * usb_gadget_controller_number - support bcdDevice id convention
> - * @gadget: the controller being driven
> - *
> - * Return a 2-digit BCD value associated with the peripheral controller,
> - * suitable for use as part of a bcdDevice value, or a negative error code.
> - *
> - * NOTE:  this convention is purely optional, and has no meaning in terms of
> - * any USB specification.  If you want to use a different convention in your
> - * gadget driver firmware -- maybe a more formal revision ID -- feel free.
> - *
> - * Hosts see these bcdDevice numbers, and are allowed (but not encouraged!)
> - * to change their behavior accordingly.  For example it might help avoiding
> - * some chip bug.
> - */
> -static inline int usb_gadget_controller_number(struct usb_gadget *gadget)
> -{
> - if (gadget_is_net2280(gadget))
> - return 0x01;
> - else if (gadget_is_dummy(gadget))
> - return 0x02;
> - else if (gadget_is_sh(gadget))
> - return 0x04;
> - else if (gadget_is_goku(gadget))
> - return 0x06;
> - else if (gadget_is_mq11xx(gadget))
> - return 0x07;
> - else if (gadget_is_omap(gadget))
> - return 0x08;
> - else if (gadget_is_n9604(gadget))
> - return 0x09;
> - else if (gadget_is_at91(gadget))
> - return 0x12;
> - else if (gadget_is_imx(gadget))
> - return 0x13;
> - else if (gadget_is_musbhsfc(gadget))
> - return 0x14;
> - else if (gadget_is_musbhdrc(gadget))
> - return 0x15;
> - else if (gadget_is_atmel_usba(gadget))
> - return 0x17;
> - else if (gadget_is_fsl_usb2(gadget))
> - return 0x18;
> - else if (gadget_is_amd5536udc(gadget))
> - return 0x19;
> - else if (gadget_is_m66592(gadget))
> - return 0x20;
> - else if (gadget_is_ci(gadget))
> - return 0x21;
> - else if (gadget_is_dwc3(gadget))
> - return 0x23;
> - else if (gadget_is_cdns3(gadget))
> - return 0x24;
> - else if (gadget_is_max3420(gadget))
> - return 0x25;
> - else if (gadget_is_mtu3(gadget))
> - return 0x26;
> - else if (gadget_is_dwc2(gadget))
> - return 0x27;
> - return -ENOENT;
> -}
> -- 
> 2.43.0


Re: [PATCH 2/6] usb: gadget: ether: Drop usb_gadget_controller_number()

2024-06-11 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On dim., juin 09, 2024 at 23:32, Marek Vasut  
wrote:

> The bcdDevice field is defined as
> |Device release number in binary-coded decimal
> in the USB 2.0 specification. We use this field to distinguish the UDCs
> from each other. In theory this could be used on the host side to apply
> certain quirks if the "special" UDC in combination with this gadget is
> used. This hasn't been done as far as I am aware. In practice it would
> be better to fix the UDC driver before shipping since a later release
> might not need this quirk anymore.
>
> This patch converts this gadget to use the U-Boot version instead of a
> random 2 or 3 plus the UDC number. Linux stopped using this functionality
> in 2012, remove it from U-Boot as well.
>
> Matching Linux kernel commit:
> ed9cbda63d45 ("usb: gadget: remove usb_gadget_controller_number()")
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Mattijs Korpershoek 
Tested-by: Mattijs Korpershoek  # on vim3

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/ether.c | 17 +++--
>  1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> index b8b29d399b1..e76464e121b 100644
> --- a/drivers/usb/gadget/ether.c
> +++ b/drivers/usb/gadget/ether.c
> @@ -22,8 +22,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
> -#include "gadget_chips.h"
>  #include "rndis.h"
>  
>  #include 
> @@ -1998,19 +1998,8 @@ static int eth_bind(struct usb_gadget *gadget)
>   rndis = 0;
>   }
>  
> - gcnum = usb_gadget_controller_number(gadget);
> - if (gcnum >= 0)
> - device_desc.bcdDevice = cpu_to_le16(0x0300 + gcnum);
> - else {
> - /*
> -  * can't assume CDC works.  don't want to default to
> -  * anything less functional on CDC-capable hardware,
> -  * so we fail in this case.
> -  */
> - pr_err("controller '%s' not recognized",
> - gadget->name);
> - return -ENODEV;
> - }
> + gcnum = (U_BOOT_VERSION_NUM << 4) | U_BOOT_VERSION_NUM_PATCH;
> + device_desc.bcdDevice = cpu_to_le16(gcnum);
>  
>   /*
>* If there's an RNDIS configuration, that's what Windows wants to
> -- 
> 2.43.0


Re: [PATCH 1/6] usb: gadget: g_dnl: Drop usb_gadget_controller_number()

2024-06-11 Thread Mattijs Korpershoek
Hi Marek,

Thank you for the patch.

On dim., juin 09, 2024 at 23:32, Marek Vasut  
wrote:

> The bcdDevice field is defined as
> |Device release number in binary-coded decimal
> in the USB 2.0 specification. We use this field to distinguish the UDCs
> from each other. In theory this could be used on the host side to apply
> certain quirks if the "special" UDC in combination with this gadget is
> used. This hasn't been done as far as I am aware. In practice it would
> be better to fix the UDC driver before shipping since a later release
> might not need this quirk anymore.
>
> This patch converts this gadget to use the U-Boot version instead of a
> random 2 or 3 plus the UDC number. Linux stopped using this functionality
> in 2012, remove it from U-Boot as well.
>
> Matching Linux kernel commit:
> ed9cbda63d45 ("usb: gadget: remove usb_gadget_controller_number()")
>
> Signed-off-by: Marek Vasut 

Compared with linux commit, and looks good to me.

Reviewed-by: Mattijs Korpershoek 

Tested that I could use fastboot, ums and scan for storage devices on
khadas vim3

Tested-by: Mattijs Korpershoek  # vim3

> ---
> Cc: Alexander Sverdlin 
> Cc: Felipe Balbi 
> Cc: Lukasz Majewski 
> Cc: Mattijs Korpershoek 
> Cc: Nishanth Menon 
> Cc: Simon Glass 
> Cc: Thinh Nguyen 
> Cc: Tom Rini 
> Cc: u-boot@lists.denx.de
> ---
>  drivers/usb/gadget/g_dnl.c | 17 +++--
>  1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
> index b5b5f5d8c11..631969b3405 100644
> --- a/drivers/usb/gadget/g_dnl.c
> +++ b/drivers/usb/gadget/g_dnl.c
> @@ -17,10 +17,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> -#include "gadget_chips.h"
>  #include "composite.c"
>  
>  /*
> @@ -199,18 +199,6 @@ void g_dnl_clear_detach(void)
>   g_dnl_detach_request = false;
>  }
>  
> -static int g_dnl_get_bcd_device_number(struct usb_composite_dev *cdev)
> -{
> - struct usb_gadget *gadget = cdev->gadget;
> - int gcnum;
> -
> - gcnum = usb_gadget_controller_number(gadget);
> - if (gcnum > 0)
> - gcnum += 0x200;
> -
> - return g_dnl_get_board_bcd_device_number(gcnum);
> -}
> -
>  /**
>   * Update internal serial number variable when the "serial#" env var changes.
>   *
> @@ -261,7 +249,8 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
>   if (ret)
>   goto error;
>  
> - gcnum = g_dnl_get_bcd_device_number(cdev);
> + gcnum = g_dnl_get_board_bcd_device_number((U_BOOT_VERSION_NUM << 4) |
> +   U_BOOT_VERSION_NUM_PATCH);
>   if (gcnum >= 0)
>   device_desc.bcdDevice = cpu_to_le16(gcnum);
>   else {
> -- 
> 2.43.0


Re: [PATCH 2/3] arm: dts: am625_sk: Switch to OF_UPSTREAM

2024-06-07 Thread Mattijs Korpershoek
Hi Nishanth,

Thank you for the patch.

On mer., juin 05, 2024 at 10:27, Nishanth Menon  wrote:

> Enable OF_UPSTREAM for am625-sk board. Remove DT files that
> are now available in dts/upstream. Update the appended files based on
> version of latest OF_UPSTREAM sync point (v6.10-rc1).
>
> Signed-off-by: Nishanth Menon 

Reviewed-by: Mattijs Korpershoek 

Boot tested to main U-Boot via DFU on AM62X SK EVM.

Tested-by: Mattijs Korpershoek 

> ---
>  arch/arm/dts/Makefile|3 +-
>  arch/arm/dts/k3-am62-main.dtsi   | 1058 --
>  arch/arm/dts/k3-am62-mcu.dtsi|  176 -
>  arch/arm/dts/k3-am62-thermal.dtsi|   36 -
>  arch/arm/dts/k3-am62-wakeup.dtsi |   96 ---
>  arch/arm/dts/k3-am62.dtsi|  122 ---
>  arch/arm/dts/k3-am625-sk-binman.dtsi |2 +-
>  arch/arm/dts/k3-am625-sk.dts |  299 
>  arch/arm/dts/k3-am625.dtsi   |  155 
>  arch/arm/dts/k3-am62x-sk-common.dtsi |  535 -
>  configs/am62x_evm_a53_defconfig  |3 +-
>  11 files changed, 4 insertions(+), 2481 deletions(-)
>  delete mode 100644 arch/arm/dts/k3-am62-main.dtsi
>  delete mode 100644 arch/arm/dts/k3-am62-mcu.dtsi
>  delete mode 100644 arch/arm/dts/k3-am62-thermal.dtsi
>  delete mode 100644 arch/arm/dts/k3-am62-wakeup.dtsi
>  delete mode 100644 arch/arm/dts/k3-am62.dtsi
>  delete mode 100644 arch/arm/dts/k3-am625-sk.dts
>  delete mode 100644 arch/arm/dts/k3-am625.dtsi
>  delete mode 100644 arch/arm/dts/k3-am62x-sk-common.dtsi
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 813426a3e519..5b0bcf336924 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -1198,8 +1198,7 @@ dtb-$(CONFIG_SOC_K3_AM642) += k3-am642-r5-evm.dtb \
> k3-am642-r5-sk.dtb \
> k3-am642-r5-phycore-som-2gb.dtb
>  
> -dtb-$(CONFIG_SOC_K3_AM625) += k3-am625-sk.dtb \
> -   k3-am625-r5-sk.dtb \
> +dtb-$(CONFIG_SOC_K3_AM625) += k3-am625-r5-sk.dtb \
> k3-am625-r5-beagleplay.dtb \
> k3-am625-verdin-r5.dtb \
> k3-am625-r5-phycore-som-2gb.dtb
> diff --git a/arch/arm/dts/k3-am62-main.dtsi b/arch/arm/dts/k3-am62-main.dtsi
> deleted file mode 100644
> index e9cffca073ef..
> --- a/arch/arm/dts/k3-am62-main.dtsi
> +++ /dev/null
> @@ -1,1058 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only OR MIT
> -/*
> - * Device Tree Source for AM625 SoC Family Main Domain peripherals
> - *
> - * Copyright (C) 2020-2024 Texas Instruments Incorporated - 
> https://www.ti.com/
> - */
> -
> -_main {
> - oc_sram: sram@7000 {
> - compatible = "mmio-sram";
> - reg = <0x00 0x7000 0x00 0x1>;
> - #address-cells = <1>;
> - #size-cells = <1>;
> - ranges = <0x0 0x00 0x7000 0x1>;
> - };
> -
> - gic500: interrupt-controller@180 {
> - compatible = "arm,gic-v3";
> - #address-cells = <2>;
> - #size-cells = <2>;
> - ranges;
> - #interrupt-cells = <3>;
> - interrupt-controller;
> - reg = <0x00 0x0180 0x00 0x1>,   /* GICD */
> -   <0x00 0x0188 0x00 0xc>,   /* GICR */
> -   <0x00 0x0188 0x00 0xc>,   /* GICR */
> -   <0x01 0x 0x00 0x2000>,/* GICC */
> -   <0x01 0x0001 0x00 0x1000>,/* GICH */
> -   <0x01 0x0002 0x00 0x2000>;/* GICV */
> - /*
> -  * vcpumntirq:
> -  * virtual CPU interface maintenance interrupt
> -  */
> - interrupts = ;
> -
> - gic_its: msi-controller@182 {
> - compatible = "arm,gic-v3-its";
> - reg = <0x00 0x0182 0x00 0x1>;
> - socionext,synquacer-pre-its = <0x100 0x40>;
> - msi-controller;
> - #msi-cells = <1>;
> - };
> - };
> -
> - main_conf: bus@10 {
> - compatible = "simple-bus";
> - #address-cells = <1>;
> - #size-cells = <1>;
> - ranges = <0x0 0x00 0x0010 0x2>;
> -
> - phy_gmii_sel: phy@4044 {
> - compatible = "ti,am654-phy-gmii-sel";
> - reg = <0x4044 0x8>;
> - 

[PATCH 5/6] bootstd: Add a bootmeth for Android

2024-06-06 Thread Mattijs Korpershoek
Android boot flow is a bit different than a regular Linux distro.
Android relies on multiple partitions in order to boot.

A typical boot flow would be:
1. Parse the Bootloader Control Block (BCB, misc partition)
2. If BCB requested bootonce-bootloader, start fastboot and wait.
3. If BCB requested recovery or normal android, run the following:
3.a. Get slot (A/B) from BCB
3.b. Run AVB (Android Verified Boot) on boot partitions
3.c. Load boot and vendor_boot partitions
3.d. Load device-tree, ramdisk and boot

The AOSP documentation has more details at [1], [2], [3]

This has been implemented via complex boot scripts such as [4].
However, these boot script are neither very maintainable nor generic.
Moreover, DISTRO_DEFAULTS is being deprecated [5].

Add a generic Android bootflow implementation for bootstd.
For this initial version, only boot image v4 is supported.

[1] https://source.android.com/docs/core/architecture/bootloader
[2] https://source.android.com/docs/core/architecture/partitions
[3] https://source.android.com/docs/core/architecture/partitions/generic-boot
[4] 
https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_android.h
[5] https://lore.kernel.org/r/all/20230914165615.1058529-17-...@chromium.org/

Signed-off-by: Mattijs Korpershoek 
---
 MAINTAINERS |   7 +
 boot/Kconfig|  14 ++
 boot/Makefile   |   2 +
 boot/bootmeth_android.c | 522 
 boot/bootmeth_android.h |  27 +++
 doc/develop/bootstd.rst |   6 +
 6 files changed, 578 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 66783d636e3d..6d2b87720565 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -939,6 +939,13 @@ F: include/bootstd.h
 F: net/eth_bootdevice.c
 F: test/boot/
 
+BOOTMETH_ANDROID
+M: Mattijs Korpershoek 
+S: Maintained
+T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git
+F: boot/bootmeth_android.c
+F: boot/bootmeth_android.h
+
 BTRFS
 M: Marek Behún 
 R: Qu Wenruo 
diff --git a/boot/Kconfig b/boot/Kconfig
index 6f3096c15a6f..5fa6f3b8315d 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -494,6 +494,20 @@ config BOOTMETH_GLOBAL
  EFI bootmgr, since they take full control over which bootdevs are
  selected to boot.
 
+config BOOTMETH_ANDROID
+   bool "Bootdev support for Android"
+   depends on X86 || ARM || SANDBOX
+   select ANDROID_AB
+   select ANDROID_BOOT_IMAGE
+   select CMD_BCB
+   select PARTITION_TYPE_GUID
+   select PARTITION_UUIDS
+   help
+ Enables support for booting Android using bootdevs. Android requires
+ multiple partitions (misc, boot, vbmeta, ...) in storage for booting.
+
+ Note that only MMC bootdevs are supported at present.
+
 config BOOTMETH_CROS
bool "Bootdev support for Chromium OS"
depends on X86 || ARM || SANDBOX
diff --git a/boot/Makefile b/boot/Makefile
index 84ccfeaecec4..75d1cd46fabf 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -66,3 +66,5 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_REQUEST) += vbe_request.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_OS) += vbe_simple_os.o
+
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_ANDROID) += bootmeth_android.o
diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
new file mode 100644
index ..26d548d2fd6e
--- /dev/null
+++ b/boot/bootmeth_android.c
@@ -0,0 +1,522 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Bootmethod for Android
+ *
+ * Copyright (C) 2024 BayLibre, SAS
+ * Written by Mattijs Korpershoek 
+ */
+#define LOG_CATEGORY UCLASS_BOOTSTD
+
+#include 
+#include 
+#if CONFIG_IS_ENABLED(AVB_VERIFY)
+#include 
+#endif
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "bootmeth_android.h"
+
+#define BCB_FIELD_COMMAND_SZ 32
+#define BCB_PART_NAME "misc"
+#define BOOT_PART_NAME "boot"
+#define VENDOR_BOOT_PART_NAME "vendor_boot"
+
+/**
+ * struct android_priv - Private data
+ *
+ * This is read from the disk and recorded for use when the full Android
+ * kernel must be loaded and booted
+ */
+struct android_priv {
+   int boot_mode;
+   char slot[2];
+   u32 header_version;
+};
+
+static int android_check(struct udevice *dev, struct bootflow_iter *iter)
+{
+   /* This only works on mmc devices */
+   if (bootflow_iter_check_mmc(iter))
+   return log_msg_ret("mmc", -ENOTSUPP);
+
+   /* This only works on whole devices, as multiple
+* partitions are needed to boot Android
+*/
+   if (iter->part != 0)
+   return log_msg_ret("mmc part", -ENOTSUPP);
+
+   return 0;
+}
+
+static int scan_boot_part(struct udevice *blk, struct android_priv *priv)
+{
+   

[PATCH 6/6] bootstd: Add test for bootmeth_android

2024-06-06 Thread Mattijs Korpershoek
Add a unit test for testing the Android bootmethod.

This requires another mmc image (mmc7) to contain the following partitions:
- misc: contains the Bootloader Control Block (BCB)
- boot_a: contains a fake generic kernel image
- vendor_boot_a: contains a fake vendor_boot image

Also add BOOTMETH_ANDROID as a dependency on sandbox so that we can test
this with:

$ ./test/py/test.py --bd sandbox --build -k test_ut # to build the mmc7.img
$ ./test/py/test.py --bd sandbox --build -k bootflow_android

Signed-off-by: Mattijs Korpershoek 
---
 arch/sandbox/dts/test.dts |  8 +
 configs/sandbox_defconfig |  2 +-
 test/boot/bootflow.c  | 65 ++--
 test/py/tests/test_ut.py  | 76 +++
 4 files changed, 147 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index a012f5c4c9ba..5fb5eac862ec 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -43,6 +43,7 @@
mmc4 = "/mmc4";
mmc5 = "/mmc5";
mmc6 = "/mmc6";
+   mmc7 = "/mmc7";
pci0 = 
pci1 = 
pci2 = 
@@ -1129,6 +1130,13 @@
filename = "mmc6.img";
};
 
+   /* This is used for Android tests */
+   mmc7 {
+   status = "disabled";
+   compatible = "sandbox,mmc";
+   filename = "mmc7.img";
+   };
+
pch {
compatible = "sandbox,pch";
};
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 93b52f2de5cf..bc4398f101a7 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -15,6 +15,7 @@ CONFIG_FIT=y
 CONFIG_FIT_RSASSA_PSS=y
 CONFIG_FIT_CIPHER=y
 CONFIG_FIT_VERBOSE=y
+CONFIG_BOOTMETH_ANDROID=y
 CONFIG_LEGACY_IMAGE_FORMAT=y
 CONFIG_MEASURED_BOOT=y
 CONFIG_BOOTSTAGE=y
@@ -40,7 +41,6 @@ CONFIG_LOG_MAX_LEVEL=9
 CONFIG_LOG_DEFAULT_LEVEL=6
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_STACKPROTECTOR=y
-CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_SMBIOS=y
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 4511cfa7f9bf..934c4dcbad2b 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -27,6 +27,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+extern U_BOOT_DRIVER(bootmeth_android);
 extern U_BOOT_DRIVER(bootmeth_cros);
 extern U_BOOT_DRIVER(bootmeth_2script);
 
@@ -518,12 +519,12 @@ BOOTSTD_TEST(bootflow_cmd_boot, UT_TESTF_DM | 
UT_TESTF_SCAN_FDT);
  * @uts: Unit test state
  * @mmc_dev: MMC device to use, e.g. "mmc4". Note that this must remain valid
  * in the caller until
- * @bind_cros: true to bind the ChromiumOS bootmeth
+ * @bind_cros: true to bind the ChromiumOS and Android bootmeths
  * @old_orderp: Returns the original bootdev order, which must be restored
  * Returns 0 on success, -ve on failure
  */
 static int prep_mmc_bootdev(struct unit_test_state *uts, const char *mmc_dev,
-   bool bind_cros, const char ***old_orderp)
+   bool bind_cros_android, const char ***old_orderp)
 {
static const char *order[] = {"mmc2", "mmc1", NULL, NULL};
struct udevice *dev, *bootstd;
@@ -545,12 +546,19 @@ static int prep_mmc_bootdev(struct unit_test_state *uts, 
const char *mmc_dev,
"bootmeth_script", 0, ofnode_null(), ));
 
/* Enable the cros bootmeth if needed */
-   if (IS_ENABLED(CONFIG_BOOTMETH_CROS) && bind_cros) {
+   if (IS_ENABLED(CONFIG_BOOTMETH_CROS) && bind_cros_android) {
ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, ));
ut_assertok(device_bind(bootstd, DM_DRIVER_REF(bootmeth_cros),
"cros", 0, ofnode_null(), ));
}
 
+   /* Enable the android bootmeths if needed */
+   if (IS_ENABLED(CONFIG_BOOTMETH_ANDROID) && bind_cros_android) {
+   ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, ));
+   ut_assertok(device_bind(bootstd, 
DM_DRIVER_REF(bootmeth_android),
+   "android", 0, ofnode_null(), ));
+   }
+
/* Change the order to include the device */
std = dev_get_priv(bootstd);
old_order = std->bootdev_order;
@@ -589,6 +597,37 @@ static int scan_mmc_bootdev(struct unit_test_state *uts, 
const char *mmc_dev,
return 0;
 }
 
+/**
+ * scan_mmc_android_bootdev() - Set up an mmc bootdev so we can access other
+ * distros. Android bootflow might print "ANDROID:*" while scanning
+ *
+ * @uts: Unit test state
+ * @mmc_dev: MMC device to use, e.g. "mmc4"
+ * Returns 0 on success, -ve on failure
+ */
+static int scan_mmc_android_bootdev(struct unit_test_stat

[PATCH 4/6] android: boot: Add set_abootimg_addr() and set_avendor_bootimg_addr()

2024-06-06 Thread Mattijs Korpershoek
The only way to configure the load addresses for both bootimg and
vendor_bootimg is by using the "abootimg" command.
If we want to use the C API, there is no equivalent.

Add set_abootimg_addr() and set_avendor_bootimg_addr() so that we can
specify the load address from C.

This can be useful for implementing an Android bootmethod.

Signed-off-by: Mattijs Korpershoek 
---
 cmd/abootimg.c  | 10 ++
 include/image.h | 14 ++
 2 files changed, 24 insertions(+)

diff --git a/cmd/abootimg.c b/cmd/abootimg.c
index 88c77d999290..33381e22dec2 100644
--- a/cmd/abootimg.c
+++ b/cmd/abootimg.c
@@ -21,11 +21,21 @@ ulong get_abootimg_addr(void)
return (_abootimg_addr == -1 ? image_load_addr : _abootimg_addr);
 }
 
+void set_abootimg_addr(ulong addr)
+{
+   _abootimg_addr = addr;
+}
+
 ulong get_avendor_bootimg_addr(void)
 {
return _avendor_bootimg_addr;
 }
 
+void set_avendor_bootimg_addr(ulong addr)
+{
+   _avendor_bootimg_addr = addr;
+}
+
 static int abootimg_get_ver(int argc, char *const argv[])
 {
const struct andr_boot_img_hdr_v0 *hdr;
diff --git a/include/image.h b/include/image.h
index 18e5ced5ab42..6deaf406605e 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1977,6 +1977,13 @@ u32 android_image_get_version(const void *hdr);
  */
 ulong get_abootimg_addr(void);
 
+/**
+ * set_abootimg_addr() - Set Android boot image address
+ *
+ * Return: no returned results
+ */
+void set_abootimg_addr(ulong addr);
+
 /**
  * get_avendor_bootimg_addr() - Get Android vendor boot image address
  *
@@ -1984,6 +1991,13 @@ ulong get_abootimg_addr(void);
  */
 ulong get_avendor_bootimg_addr(void);
 
+/**
+ * set_abootimg_addr() - Set Android vendor boot image address
+ *
+ * Return: no returned results
+ */
+void set_avendor_bootimg_addr(ulong addr);
+
 /**
  * board_fit_config_name_match() - Check for a matching board name
  *

-- 
2.45.0



[PATCH 3/6] bootstd: Add bootflow_iter_check_mmc() helper

2024-06-06 Thread Mattijs Korpershoek
Some bootflows might be able to only boot from MMC devices.

Add a helper function these bootflows can use.

Signed-off-by: Mattijs Korpershoek 
---
 boot/bootflow.c| 12 
 include/bootflow.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/boot/bootflow.c b/boot/bootflow.c
index 9aa3179c3881..59d77d2385f4 100644
--- a/boot/bootflow.c
+++ b/boot/bootflow.c
@@ -575,6 +575,18 @@ int bootflow_iter_check_blk(const struct bootflow_iter 
*iter)
return -ENOTSUPP;
 }
 
+int bootflow_iter_check_mmc(const struct bootflow_iter *iter)
+{
+   const struct udevice *media = dev_get_parent(iter->dev);
+   enum uclass_id id = device_get_uclass_id(media);
+
+   log_debug("uclass %d: %s\n", id, uclass_get_name(id));
+   if (id == UCLASS_MMC)
+   return 0;
+
+   return -ENOTSUPP;
+}
+
 int bootflow_iter_check_sf(const struct bootflow_iter *iter)
 {
const struct udevice *media = dev_get_parent(iter->dev);
diff --git a/include/bootflow.h b/include/bootflow.h
index 080ee8501225..6058ddd89b16 100644
--- a/include/bootflow.h
+++ b/include/bootflow.h
@@ -407,6 +407,15 @@ void bootflow_remove(struct bootflow *bflow);
  */
 int bootflow_iter_check_blk(const struct bootflow_iter *iter);
 
+/**
+ * bootflow_iter_check_mmc() - Check that a bootflow uses a MMC device
+ *
+ * This checks the bootdev in the bootflow to make sure it uses a mmc device
+ *
+ * Return: 0 if OK, -ENOTSUPP if some other device is used (e.g. ethernet)
+ */
+int bootflow_iter_check_mmc(const struct bootflow_iter *iter);
+
 /**
  * bootflow_iter_check_sf() - Check that a bootflow uses SPI FLASH
  *

-- 
2.45.0



[PATCH 2/6] boot: android: Add image_android_get_version()

2024-06-06 Thread Mattijs Korpershoek
When reading a boot image header, we may need to retrieve the header
version.

Add a helper function for it.

Signed-off-by: Mattijs Korpershoek 
---
 boot/image-android.c | 7 ++-
 include/image.h  | 7 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index ddd8ffd5e540..4f8fb51585eb 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -185,7 +185,7 @@ bool android_image_get_data(const void *boot_hdr, const 
void *vendor_boot_hdr,
return false;
}
 
-   if (((struct andr_boot_img_hdr_v0 *)boot_hdr)->header_version > 2) {
+   if (android_image_get_version(boot_hdr) > 2) {
if (!vendor_boot_hdr) {
printf("For boot header v3+ vendor boot image has to be 
provided\n");
return false;
@@ -203,6 +203,11 @@ bool android_image_get_data(const void *boot_hdr, const 
void *vendor_boot_hdr,
return true;
 }
 
+u32 android_image_get_version(const void *hdr)
+{
+   return ((struct andr_boot_img_hdr_v0 *)hdr)->header_version;
+}
+
 static ulong android_image_get_kernel_addr(struct andr_image_data *img_data)
 {
/*
diff --git a/include/image.h b/include/image.h
index acffd17e0dfd..18e5ced5ab42 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1963,6 +1963,13 @@ bool is_android_boot_image_header(const void *hdr);
  */
 bool is_android_vendor_boot_image_header(const void *vendor_boot_img);
 
+/**
+ * android_image_get_version() - Retrieve the boot.img version
+ *
+ * Return: Android boot image header version.
+ */
+u32 android_image_get_version(const void *hdr);
+
 /**
  * get_abootimg_addr() - Get Android boot image address
  *

-- 
2.45.0



[PATCH 1/6] boot: android: Provide vendor_bootimg_addr in boot_get_fdt()

2024-06-06 Thread Mattijs Korpershoek
When calling android_image_get_dtb_by_index() using boot image v3+,
we should also pass the vendor_boot ramdisk address.

Use get_avendor_bootimg_addr() to do so.

Note: on boot image v2, this is harmless since get_avendor_bootimg_addr()
returns -1.

Signed-off-by: Mattijs Korpershoek 
---
 boot/image-fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index 56dd7687f51c..8332792b8e80 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -502,7 +502,7 @@ int boot_get_fdt(void *buf, const char *select, uint arch,
 * Firstly check if this android boot image has dtb field.
 */
dtb_idx = (u32)env_get_ulong("adtb_idx", 10, 0);
-   if (android_image_get_dtb_by_index((ulong)hdr, 0,
+   if (android_image_get_dtb_by_index((ulong)hdr, 
get_avendor_bootimg_addr(),
   dtb_idx, _addr, 
_size)) {
fdt_blob = (char *)map_sysmem(fdt_addr, 0);
if (fdt_check_header(fdt_blob))

-- 
2.45.0



[PATCH 0/6] bootstd: Add Android support

2024-06-06 Thread Mattijs Korpershoek
Android boot flow is a bit different than a regular Linux distro.
Android relies on multiple partitions in order to boot.

A typical boot flow would be:
1. Parse the Bootloader Control Block (BCB, misc partition)
2. If BCB requested bootonce-bootloader, start fastboot and wait.
3. If BCB requested recovery or normal android, run the following:
   a. Get slot (A/B) from BCB
   b. Run AVB (Android Verified Boot) on boot partitions
   c. Load boot and vendor_boot partitions
   d. Load device-tree, ramdisk and boot

The AOSP documentation has more details at [1], [2], [3]

This has been implemented via complex boot scripts such as [4].
However, these boot script are neither very maintainable nor generic.
Moreover, DISTRO_DEFAULTS is being deprecated [5].

Add a generic Android bootflow implementation for bootstd.

For this initial version, only boot image v4 is supported.

This has been tested on sandbox using:
$ ./test/py/test.py --bd sandbox --build -k test_ut

This has also been tested on the AM62X SK EVM using TI's Android SDK[6]
To test on TI board, the following (WIP) patch is needed as well:
https://gitlab.baylibre.com/baylibre/ti/ti-u-boot/-/commit/84cceb912bccd7cdd7f9dd69bca0e5d987a1fd04

[1] https://source.android.com/docs/core/architecture/bootloader
[2] https://source.android.com/docs/core/architecture/partitions
[3] https://source.android.com/docs/core/architecture/partitions/generic-boot
[4] 
https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_android.h
[5] https://lore.kernel.org/r/all/20230914165615.1058529-17-...@chromium.org/
[6] 
https://software-dl.ti.com/processor-sdk-android/esd/AM62X/09_02_00/docs/android/Overview.html

Signed-off-by: Mattijs Korpershoek 
---
Mattijs Korpershoek (6):
  boot: android: Provide vendor_bootimg_addr in boot_get_fdt()
  boot: android: Add image_android_get_version()
  bootstd: Add bootflow_iter_check_mmc() helper
  android: boot: Add set_abootimg_addr() and set_avendor_bootimg_addr()
  bootstd: Add a bootmeth for Android
  bootstd: Add test for bootmeth_android

 MAINTAINERS   |   7 +
 arch/sandbox/dts/test.dts |   8 +
 boot/Kconfig  |  14 ++
 boot/Makefile |   2 +
 boot/bootflow.c   |  12 ++
 boot/bootmeth_android.c   | 522 ++
 boot/bootmeth_android.h   |  27 +++
 boot/image-android.c  |   7 +-
 boot/image-fdt.c  |   2 +-
 cmd/abootimg.c|  10 +
 configs/sandbox_defconfig |   2 +-
 doc/develop/bootstd.rst   |   6 +
 include/bootflow.h|   9 +
 include/image.h   |  21 ++
 test/boot/bootflow.c  |  65 +-
 test/py/tests/test_ut.py  |  76 +++
 16 files changed, 784 insertions(+), 6 deletions(-)
---
base-commit: 227be29df37545f74243a98c12a4a33c4160e3cd
change-id: 20240605-bootmeth-android-bfc8596e9367

Best regards,
-- 
Mattijs Korpershoek 



Re: [GIT PULL] Please pull u-boot-dfu-20240606

2024-06-06 Thread Mattijs Korpershoek
Hi everyone,

On jeu., mai 16, 2024 at 17:12, Mattijs Korpershoek  
wrote:

> Hi Tom,
>
> Please find some fixes for master:
>
> - dwc3 fix crash when ep0 stalls or gadget is stopped
> - Kconfig build fix for DFU_SF (SPI flash DFU driver)
>
> The CI job is at 
> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/20990

Please ignore this one, my mail client screwed up the send date.

Sorry about that :(

Mattijs

>
> Thanks,
> Mattijs
>
> The following changes since commit c0ea27bccfb7d2d37fd36806ac2a2f7389099420:
>
>   Prepare v2024.07-rc4 (2024-06-03 18:34:59 -0600)
>
> are available in the Git repository at:
>
>   https://source.denx.de/u-boot/custodians/u-boot-dfu.git 
> tags/u-boot-dfu-20240606
>
> for you to fetch changes up to 4339138a2086f8449b9356130cb6e97a81aa8679:
>
>   dfu: add missing dependency for SPI flash DFU driver (2024-06-06 09:11:21 
> +0200)
>
> 
> u-boot-dfu-20240606
>
> - dwc3 fix crash when ep0 stalls or gadget is stopped
> - Kconfig build fix for DFU_SF (SPI flash DFU driver)
>
> 
> Heinrich Schuchardt (1):
>   dfu: add missing dependency for SPI flash DFU driver
>
> Neil Armstrong (1):
>   usb: dwc3: gadget: fix crash in dwc3_gadget_giveback()
>
>  drivers/dfu/Kconfig   | 1 +
>  drivers/usb/dwc3/gadget.c | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)


[GIT PULL] Please pull u-boot-dfu-20240606

2024-06-06 Thread Mattijs Korpershoek
Hi Tom,

Please find some fixes for master:

- dwc3 fix crash when ep0 stalls or gadget is stopped
- Kconfig build fix for DFU_SF (SPI flash DFU driver)

The CI job is at 
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/20990

Thanks,
Mattijs

The following changes since commit c0ea27bccfb7d2d37fd36806ac2a2f7389099420:

  Prepare v2024.07-rc4 (2024-06-03 18:34:59 -0600)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-dfu.git 
tags/u-boot-dfu-20240606

for you to fetch changes up to 4339138a2086f8449b9356130cb6e97a81aa8679:

  dfu: add missing dependency for SPI flash DFU driver (2024-06-06 09:11:21 
+0200)


u-boot-dfu-20240606

- dwc3 fix crash when ep0 stalls or gadget is stopped
- Kconfig build fix for DFU_SF (SPI flash DFU driver)


Heinrich Schuchardt (1):
  dfu: add missing dependency for SPI flash DFU driver

Neil Armstrong (1):
  usb: dwc3: gadget: fix crash in dwc3_gadget_giveback()

 drivers/dfu/Kconfig   | 1 +
 drivers/usb/dwc3/gadget.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)


[GIT PULL] Please pull u-boot-dfu-20240606

2024-06-06 Thread Mattijs Korpershoek
Hi Tom,

Please find some fixes for master:

- dwc3 fix crash when ep0 stalls or gadget is stopped
- Kconfig build fix for DFU_SF (SPI flash DFU driver)

The CI job is at 
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/20990

Thanks,
Mattijs

The following changes since commit c0ea27bccfb7d2d37fd36806ac2a2f7389099420:

  Prepare v2024.07-rc4 (2024-06-03 18:34:59 -0600)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-dfu.git 
tags/u-boot-dfu-20240606

for you to fetch changes up to 4339138a2086f8449b9356130cb6e97a81aa8679:

  dfu: add missing dependency for SPI flash DFU driver (2024-06-06 09:11:21 
+0200)


u-boot-dfu-20240606

- dwc3 fix crash when ep0 stalls or gadget is stopped
- Kconfig build fix for DFU_SF (SPI flash DFU driver)


Heinrich Schuchardt (1):
  dfu: add missing dependency for SPI flash DFU driver

Neil Armstrong (1):
  usb: dwc3: gadget: fix crash in dwc3_gadget_giveback()

 drivers/dfu/Kconfig   | 1 +
 drivers/usb/dwc3/gadget.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)



Re: [PATCH 1/3] arm: dts: am625_beagleplay: Switch to OF_UPSTREAM

2024-06-06 Thread Mattijs Korpershoek
Hi Nishanth,

Thank you for the patch.

On mer., juin 05, 2024 at 10:27, Nishanth Menon  wrote:

> Enable OF_UPSTREAM for AM625-beagleplay board. Remove DT files that
> are now available in dts/upstream. Update the appended files based on
> version of latest OF_UPSTREAM sync point (v6.10-rc1).
>
> Signed-off-by: Nishanth Menon 

Reviewed-by: Mattijs Korpershoek 

> ---
>  arch/arm/dts/Makefile|   1 -
>  arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi |   4 +-
>  arch/arm/dts/k3-am625-beagleplay.dts | 932 ---
>  configs/am62x_beagleplay_a53_defconfig   |   3 +-
>  4 files changed, 4 insertions(+), 936 deletions(-)
>  delete mode 100644 arch/arm/dts/k3-am625-beagleplay.dts
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 624dadf8ece2..813426a3e519 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -1200,7 +1200,6 @@ dtb-$(CONFIG_SOC_K3_AM642) += k3-am642-r5-evm.dtb \
>  
>  dtb-$(CONFIG_SOC_K3_AM625) += k3-am625-sk.dtb \
> k3-am625-r5-sk.dtb \
> -   k3-am625-beagleplay.dtb \
> k3-am625-r5-beagleplay.dtb \
> k3-am625-verdin-r5.dtb \
> k3-am625-r5-phycore-som-2gb.dtb
> diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi 
> b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> index 9ac4a825f841..dd5b335ed2ee 100644
> --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> @@ -66,9 +66,9 @@
>  #ifdef CONFIG_TARGET_AM625_A53_BEAGLEPLAY
>  
>  #define SPL_NODTB "spl/u-boot-spl-nodtb.bin"
> -#define SPL_AM625_BEAGLEPLAY_DTB "spl/dts/k3-am625-beagleplay.dtb"
> +#define SPL_AM625_BEAGLEPLAY_DTB "spl/dts/ti/k3-am625-beagleplay.dtb"
>  #define UBOOT_NODTB "u-boot-nodtb.bin"
> -#define AM625_BEAGLEPLAY_DTB "arch/arm/dts/k3-am625-beagleplay.dtb"
> +#define AM625_BEAGLEPLAY_DTB 
> "dts/upstream/src/arm64/ti/k3-am625-beagleplay.dtb"
>  
>   {
>   ti-dm {
> diff --git a/arch/arm/dts/k3-am625-beagleplay.dts 
> b/arch/arm/dts/k3-am625-beagleplay.dts
> deleted file mode 100644
> index 8ab838f1697c..
> --- a/arch/arm/dts/k3-am625-beagleplay.dts
> +++ /dev/null
> @@ -1,932 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only OR MIT
> -/*
> - * https://beagleplay.org/
> - *
> - * Copyright (C) 2022-2024 Texas Instruments Incorporated - 
> https://www.ti.com/
> - * Copyright (C) 2022-2024 Robert Nelson, BeagleBoard.org Foundation
> - */
> -
> -/dts-v1/;
> -
> -#include 
> -#include 
> -#include 
> -#include "k3-am625.dtsi"
> -
> -/ {
> - compatible = "beagle,am625-beagleplay", "ti,am625";
> - model = "BeagleBoard.org BeaglePlay";
> -
> - aliases {
> - ethernet0 = _port1;
> - ethernet1 = _port2;
> - gpio0 = _gpio0;
> - gpio1 = _gpio1;
> - gpio2 = _gpio0;
> - i2c0 = _i2c0;
> - i2c1 = _i2c1;
> - i2c2 = _i2c2;
> - i2c3 = _i2c3;
> - i2c4 = _i2c0;
> - i2c5 = _i2c0;
> - mmc0 = 
> - mmc1 = 
> - mmc2 = 
> - rtc0 = 
> - serial0 = _uart5;
> - serial1 = _uart6;
> - serial2 = _uart0;
> - usb0 = 
> - usb1 = 
> - };
> -
> - chosen {
> - stdout-path = "serial2:115200n8";
> - };
> -
> - memory@8000 {
> - bootph-pre-ram;
> - device_type = "memory";
> - /* 2G RAM */
> - reg = <0x 0x8000 0x 0x8000>;
> - };
> -
> - reserved-memory {
> - #address-cells = <2>;
> - #size-cells = <2>;
> - ranges;
> -
> - ramoops: ramoops@9ca0 {
> - compatible = "ramoops";
> - reg = <0x00 0x9ca0 0x00 0x0010>;
> - record-size = <0x8000>;
> - console-size = <0x8000>;
> - ftrace-size = <0x00>;
> - pmsg-size = <0x8000>;
> - };
> -
> - secure_tfa_ddr: tfa@9e78 {
> - reg = <0x00 0x9e78 0x00 0x8>;
> - no-map;
> - };
> -
> - secure_ddr: optee@9e80 {
> - reg = <0x00 0x9e80 0x00 0

Re: [PATCH 1/1] dfu: add missing dependency for SPI flash DFU driver

2024-06-06 Thread Mattijs Korpershoek
Hi,

On Tue, 04 Jun 2024 07:44:25 +0200, Heinrich Schuchardt wrote:
> Building the SPI flash DFU driver fails if SPI flash support is missing.
> 
> drivers/dfu/dfu_sf.c:123:29: error:
> ‘CONFIG_SF_DEFAULT_MODE’ undeclared (first use in this function);
> 
> Add the missing dependency.
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu 
(u-boot-dfu)

[1/1] dfu: add missing dependency for SPI flash DFU driver
  
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/4339138a2086f8449b9356130cb6e97a81aa8679

--
Mattijs


Re: [PATCH 3/3] regulator: rk8xx: clarify operator precedence

2024-06-05 Thread Mattijs Korpershoek
Hi Quentin,

Thank you for the patch.

On mer., juin 05, 2024 at 11:33, Quentin Schulz  wrote:

> From: Quentin Schulz 
>
> My linter complains that the order isn't clear enough so let's put
> parentheses around the ternary condition to make it happy.
>
> Signed-off-by: Quentin Schulz 

Reviewed-by: Mattijs Korpershoek 

> ---
>  drivers/power/regulator/rk8xx.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
> index bd5a37e718f..3125835bc07 100644
> --- a/drivers/power/regulator/rk8xx.c
> +++ b/drivers/power/regulator/rk8xx.c
> @@ -520,7 +520,7 @@ static int _buck_get_enable(struct udevice *pmic, int 
> buck)
>   if (ret < 0)
>   return ret;
>  
> - return ret & mask ? true : false;
> + return (ret & mask) ? true : false;
>  }
>  
>  static int _buck_set_suspend_enable(struct udevice *pmic, int buck, bool 
> enable)
> @@ -585,7 +585,7 @@ static int _buck_get_suspend_enable(struct udevice *pmic, 
> int buck)
>   val = pmic_reg_read(pmic, RK816_REG_DCDC_SLP_EN);
>   if (val < 0)
>   return val;
> - ret = val & mask ? 1 : 0;
> + ret = (val & mask) ? 1 : 0;
>   break;
>   case RK806_ID:
>   {
> @@ -608,7 +608,7 @@ static int _buck_get_suspend_enable(struct udevice *pmic, 
> int buck)
>   val = pmic_reg_read(pmic, REG_SLEEP_SET_OFF1);
>   if (val < 0)
>   return val;
> - ret = val & mask ? 0 : 1;
> + ret = (val & mask) ? 0 : 1;
>   break;
>   case RK809_ID:
>   case RK817_ID:
> @@ -620,7 +620,7 @@ static int _buck_get_suspend_enable(struct udevice *pmic, 
> int buck)
>   val = pmic_reg_read(pmic, RK817_POWER_SLP_EN(0));
>   if (val < 0)
>   return val;
> - ret = val & mask ? 1 : 0;
> + ret = (val & mask) ? 1 : 0;
>   break;
>   default:
>   ret = -EINVAL;
> @@ -723,7 +723,7 @@ static int _ldo_get_enable(struct udevice *pmic, int ldo)
>   if (ret < 0)
>   return ret;
>  
> - return ret & mask ? true : false;
> + return (ret & mask) ? true : false;
>  }
>  
>  static int _nldo_get_enable(struct udevice *pmic, int nldo)
> @@ -980,7 +980,7 @@ static int _ldo_get_suspend_enable(struct udevice *pmic, 
> int ldo)
>   val = pmic_reg_read(pmic, RK816_REG_LDO_SLP_EN);
>   if (val < 0)
>   return val;
> - ret = val & mask ? 1 : 0;
> + ret = (val & mask) ? 1 : 0;
>   break;
>   case RK808_ID:
>   case RK818_ID:
> @@ -988,7 +988,7 @@ static int _ldo_get_suspend_enable(struct udevice *pmic, 
> int ldo)
>   val = pmic_reg_read(pmic, REG_SLEEP_SET_OFF2);
>   if (val < 0)
>   return val;
> - ret = val & mask ? 0 : 1;
> + ret = (val & mask) ? 0 : 1;
>   break;
>   case RK809_ID:
>   case RK817_ID:
> @@ -997,13 +997,13 @@ static int _ldo_get_suspend_enable(struct udevice 
> *pmic, int ldo)
>   val = pmic_reg_read(pmic, RK817_POWER_SLP_EN(0));
>   if (val < 0)
>   return val;
> - ret = val & mask ? 1 : 0;
> + ret = (val & mask) ? 1 : 0;
>   } else {
>   mask = 1 << ldo;
>   val = pmic_reg_read(pmic, RK817_POWER_SLP_EN(1));
>   if (val < 0)
>   return val;
> - ret = val & mask ? 1 : 0;
> + ret = (val & mask) ? 1 : 0;
>   }
>   break;
>   }
> @@ -1438,7 +1438,7 @@ static int switch_get_enable(struct udevice *dev)
>   if (ret < 0)
>   return ret;
>  
> - return ret & mask ? true : false;
> + return (ret & mask) ? true : false;
>  }
>  
>  static int switch_set_suspend_value(struct udevice *dev, int uvolt)
> @@ -1493,21 +1493,21 @@ static int switch_get_suspend_enable(struct udevice 
> *dev)
>   val = pmic_reg_read(dev->parent, REG_SLEEP_SET_OFF1);
>   if (val < 0)
>   return val;
> - ret = val & mask ? 0 : 1;
> + ret = (val & mask) ? 0 : 1;
>   break;
>   

Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

2024-06-05 Thread Mattijs Korpershoek
Hi Quentin,

Thank you for the review.

On mar., juin 04, 2024 at 17:25, Quentin Schulz  
wrote:

> Hi Mattijs,
>
> On 6/4/24 5:15 PM, Mattijs Korpershoek wrote:
>> According to [1], we should use bootmeth when describing the
>> struct bootmeth:
>> 
>> """
>> For version 2, a new naming scheme is used as above:
>> 
>>  - bootdev is used instead of bootdevice, because 'device' is overused,
>>  is everywhere in U-Boot, can be confused with udevice
>>  - bootmeth - because 'method' is too vanilla, appears 1300 times in
>>  U-Boot
>> """
>> 
>> Replace all occurences in various comments for consistency.
>> 
>
> s/occurences/occurrences/

Urgh, I guess I spot typos from others but not my own ones :/

>
> Sorry, was too tempting to highlight a typo in a commit that fixes typos :)
>
> (Don't send a v3 for this please :) )

I won't. I hope the person applying this patch can do a fixup.

>
>> [1] https://lore.kernel.org/u-boot/20211023232635.9195-1-...@chromium.org/
>
> Reviewed-by: Quentin Schulz 
>
> Thanks!
> Quentin


Re: [PATCH] bootstd: Fix a handful of doc typos in bootmeth

2024-06-04 Thread Mattijs Korpershoek
Hi Quentin,

Thanks for the review!

On mar., juin 04, 2024 at 14:22, Quentin Schulz  
wrote:

> Hi Mattijs,
>
> On 6/4/24 2:04 PM, Mattijs Korpershoek wrote:

[...]

>> 
>> There seems indeed to be some inconsistencies around bootmeths versus
>> bootmethods.
>> 
>> To me, we should use 'bootmeth' everywhere.
>> 
>> Simon, as the maintainer of bootflow, do you agree ?
>> 
>> I can spin up another patch to fix this.
>> 
>
> c.f. https://lore.kernel.org/u-boot/20211023232635.9195-1-...@chromium.org/
>
> """
> For version 2, a new naming scheme is used as above:
>
> - bootdev is used instead of bootdevice, because 'device' is overused,
> is everywhere in U-Boot, can be confused with udevice
> - bootmeth - because 'method' is too vanilla, appears 1300 times in
> U-Boot
> """
>
> SO I think we should change it to bootmeth(s) indeed.

Ah, thank you for the link, that is helpful.

I've done the global rename, made this into a series here:

https://lore.kernel.org/all/20240604-bootmeth-typos-v2-0-821683a95...@baylibre.com

>
> Reviewed-by: Quentin Schulz 
>
> Thanks,
> Quentin


[PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

2024-06-04 Thread Mattijs Korpershoek
According to [1], we should use bootmeth when describing the
struct bootmeth:

"""
For version 2, a new naming scheme is used as above:

- bootdev is used instead of bootdevice, because 'device' is overused,
is everywhere in U-Boot, can be confused with udevice
- bootmeth - because 'method' is too vanilla, appears 1300 times in
U-Boot
"""

Replace all occurences in various comments for consistency.

[1] https://lore.kernel.org/u-boot/20211023232635.9195-1-...@chromium.org/
Signed-off-by: Mattijs Korpershoek 
---
 board/sandbox/sandbox.env |  2 +-
 boot/bootmeth-uclass.c|  2 +-
 include/bootmeth.h| 30 +++---
 include/extlinux.h|  2 +-
 test/boot/bootflow.c  |  2 +-
 test/boot/bootmeth.c  |  6 +++---
 6 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/board/sandbox/sandbox.env b/board/sandbox/sandbox.env
index a2c19702d64d..564dce78a898 100644
--- a/board/sandbox/sandbox.env
+++ b/board/sandbox/sandbox.env
@@ -10,7 +10,7 @@ eth6addr=02:00:11:22:33:47
 ipaddr=192.0.2.1
 
 /*
- * These are used for distro boot which is not supported. But once bootmethod
+ * These are used for distro boot which is not supported. But once bootmeth
  * is provided these will be used again.
  */
 bootm_size=0x1000
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
index 1d157d54dbdd..e3475f46b34c 100644
--- a/boot/bootmeth-uclass.c
+++ b/boot/bootmeth-uclass.c
@@ -167,7 +167,7 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter, 
bool include_global)
if (pass)
iter->first_glob_method = upto;
/*
-* Get a list of bootmethods, in seq order (i.e. using
+* Get a list of bootmeths, in seq order (i.e. using
 * aliases). There may be gaps so try to count up high
 * enough to find them all.
 */
diff --git a/include/bootmeth.h b/include/bootmeth.h
index 529c4d813d82..2570d9593d49 100644
--- a/include/bootmeth.h
+++ b/include/bootmeth.h
@@ -47,7 +47,7 @@ struct bootmeth_ops {
 * This may involve reading state from the system, e.g. some data in
 * the firmware area.
 *
-* @dev:Bootmethod device to check
+* @dev:Bootmeth device to check
 * @buf:Buffer to place the info in (terminator must fit)
 * @maxsize:Size of buffer
 * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if
@@ -74,7 +74,7 @@ struct bootmeth_ops {
 *
 * It may update only the flags in @iter
 *
-* @dev:Bootmethod device to check against
+* @dev:Bootmeth device to check against
 * @iter:   On entry, provides bootdev, hwpart, part
 * Return: 0 if OK, -ENOTSUPP if this bootdev is not supported
 */
@@ -83,7 +83,7 @@ struct bootmeth_ops {
/**
 * read_bootflow() - read a bootflow for a device
 *
-* @dev:Bootmethod device to use
+* @dev:Bootmeth device to use
 * @bflow:  On entry, provides dev, hwpart, part and method.
 *  Returns updated bootflow if found
 * Return: 0 if OK, -ve on error
@@ -96,7 +96,7 @@ struct bootmeth_ops {
 * This provides a bootflow file to the bootmeth, to see if it is valid.
 * If it is, the bootflow is set up accordingly.
 *
-* @dev:Bootmethod device to use
+* @dev:Bootmeth device to use
 * @bflow:  On entry, provides bootdev.
 *  Returns updated bootflow if found
 * @buf:Buffer containing the possible bootflow file
@@ -111,7 +111,7 @@ struct bootmeth_ops {
 *
 * Read a file from the same place as the bootflow came from
 *
-* @dev:Bootmethod device to use
+* @dev:Bootmeth device to use
 * @bflow:  Bootflow providing info on where to read from
 * @file_path:  Path to file (may be absolute or relative)
 * @addr:   Address to load file
@@ -126,7 +126,7 @@ struct bootmeth_ops {
/**
 * readall() - read all files for a bootflow
 *
-* @dev:Bootmethod device to boot
+* @dev:Bootmeth device to boot
 * @bflow:  Bootflow to read
 * Return: 0 if OK, -EIO on I/O error, other -ve on other error
 */
@@ -135,7 +135,7 @@ struct bootmeth_ops {
/**
 * boot() - boot a bootflow
 *
-* @dev:Bootmethod device to boot
+* @dev:Bootmeth device to boot
 * @bflow:  Bootflow to boot
 * Return: does not return on success, since it should boot the
 *  Operating System. Returns -EFAULT if that fails, -ENOTSUPP if
@@ -15

[PATCH v2 1/2] bootstd: Fix a handful of doc typos in bootmeth

2024-06-04 Thread Mattijs Korpershoek
Fix some trivial typos found by browsing the code.
Done with flyspell.

Reviewed-by: Quentin Schulz 
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

-- 
2.45.0



[PATCH v2 0/2] bootstd: Fix a handful of doc typos in bootmeth

2024-06-04 Thread Mattijs Korpershoek
While working on bootflow, we noticed some wording inconsistencies with
bootmeth versus bootmethod.

According to [1], we should use bootmeth(s), not bootmethod(s):

"""
For version 2, a new naming scheme is used as above:

- bootdev is used instead of bootdevice, because 'device' is overused,
is everywhere in U-Boot, can be confused with udevice
- bootmeth - because 'method' is too vanilla, appears 1300 times in
U-Boot
"""

[1] c.f. https://lore.kernel.org/u-boot/20211023232635.9195-1-...@chromium.org/

First patch fixes some trivial typos, second patch replaces all
occurences of bootmethod(s) -> bootmeth(s).

Signed-off-by: Mattijs Korpershoek 
---
Changes in v2:
- Made into a series
- New patch replaces all bootmethod(s) -> bootmeth(s)
- Link to v1: 
https://lore.kernel.org/r/20240603-bootmeth-typos-v1-1-6edbdb469...@baylibre.com

---
Mattijs Korpershoek (2):
  bootstd: Fix a handful of doc typos in bootmeth
  bootstd: Replace bootmethod(s) -> bootmeth(s)

 board/sandbox/sandbox.env |  2 +-
 boot/bootmeth-uclass.c|  2 +-
 include/bootmeth.h| 42 +-
 include/extlinux.h|  2 +-
 test/boot/bootflow.c  |  2 +-
 test/boot/bootmeth.c  |  6 +++---
 6 files changed, 28 insertions(+), 28 deletions(-)
---
base-commit: ea722aa5eb33740ae77e8816aeb72b385e621cd0
change-id: 20240603-bootmeth-typos-47c865e70ccf

Best regards,
-- 
Mattijs Korpershoek 



Re: [PATCH] bootstd: Fix a handful of doc typos in bootmeth

2024-06-04 Thread Mattijs Korpershoek
Hi Quentin,

On mar., juin 04, 2024 at 11:47, Quentin Schulz  
wrote:

> Hi Mattijs,
>
> On 6/3/24 11:11 AM, Mattijs Korpershoek wrote:
>> 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.
>
> I see we have a mix of null-terminated and nul-terminated in the tree,
> is the latter correct?

Thank you for your review.

I believe nul-terminated is correct: nul is the character, and null is the 
pointer.

See:
- https://news.ycombinator.com/item?id=22283217
- https://internals.rust-lang.org/t/null-consistency/16767

I'll check the tree and submit another patch to fix this.

>
>>*
>> @@ -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
>>*
>
> Shouldn't this be bootmeths here as well?
>
> (And there's another occurrence in boot/bootmeth-uclass.c

There seems indeed to be some inconsistencies around bootmeths versus
bootmethods.

To me, we should use 'bootmeth' everywhere.

Simon, as the maintainer of bootflow, do you agree ?

I can spin up another patch to fix this.

>
> Cheers,
> Quentin


Re: [PATCH v3 1/8] dfu: add scsi backend

2024-06-04 Thread Mattijs Korpershoek
Hi Caleb,

Thank you for the patch.

On lun., juin 03, 2024 at 14:49, 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  |  32 
>  drivers/dfu/Kconfig|   7 +
>  drivers/dfu/Makefile   |   1 +
>  drivers/dfu/dfu.c  |   5 +-
>  drivers/dfu/dfu_scsi.c | 435 
> +
>  include/dfu.h  |  26 +++
>  6 files changed, 505 insertions(+), 1 deletion(-)
>
> diff --git a/doc/usage/dfu.rst b/doc/usage/dfu.rst
> index 8cc09c308d82..f497dcf137a4 100644
> --- a/doc/usage/dfu.rst
> +++ b/doc/usage/dfu.rst
> @@ -21,8 +21,9 @@ U-Boot implements this DFU capability (CONFIG_DFU) with the 
> command dfu
>  
>  Today the supported DFU backends are:
>  
>  - MMC (RAW or FAT / EXT2 / EXT3 / EXT4 file system / SKIP / SCRIPT)
> +- SCSI (UFS, RAW partition, FAT / EXT2 / EXT3 / EXT4 file system / SKIP / 
> SCRIPT)
>  - NAND
>  - RAM
>  - SF (serial flash)
>  - MTD (all MTD device: NAND, SPI-NOR, SPI-NAND,...)
> @@ -166,8 +167,38 @@ mmc
>  
>  Please note that this means the user will be able to execute any
>  arbitrary commands just like in the u-boot's shell.

Can we please add CONFIG_DFU_SCSI in "Configuration Options" section at
the beginning of this document?

See:
https://docs.u-boot.org/en/latest/usage/dfu.html#configuration-options

Note: I requested that here:
https://lore.kernel.org/all/87o7a94pe1@baylibre.com/

With above addressed, feel free to add:
Reviewed-by: Mattijs Korpershoek 

And please take this through your tree:
Acked-by: Mattijs Korpershoek 

>  
> +scsi

[...]


Re: [PATCH 1/1] dfu: add missing dependency for SPI flash DFU driver

2024-06-04 Thread Mattijs Korpershoek
Hi Heinrich,

Thank you for the patch.

On mar., juin 04, 2024 at 07:44, Heinrich Schuchardt 
 wrote:

> Building the SPI flash DFU driver fails if SPI flash support is missing.
>
> drivers/dfu/dfu_sf.c:123:29: error:
> ‘CONFIG_SF_DEFAULT_MODE’ undeclared (first use in this function);
>
> Add the missing dependency.
>
> Signed-off-by: Heinrich Schuchardt 

Reviewed-by: Mattijs Korpershoek 

> ---
>  drivers/dfu/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> index 0360d9da142..971204758aa 100644
> --- a/drivers/dfu/Kconfig
> +++ b/drivers/dfu/Kconfig
> @@ -68,6 +68,7 @@ config DFU_RAM
>  
>  config DFU_SF
>   bool "SPI flash back end for DFU"
> + depends on SPI_FLASH || DM_SPI_FLASH
>   help
> This option enables using DFU to read and write to SPI flash based
> storage.
> -- 
> 2.43.0


Re: [PATCH] usb: dwc3: gadget: fix crash in dwc3_gadget_giveback()

2024-06-04 Thread Mattijs Korpershoek
Hi,

On Tue, 28 May 2024 10:35:03 +0200, 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.
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu 
(u-boot-dfu)

[1/1] usb: dwc3: gadget: fix crash in dwc3_gadget_giveback()
  
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/85ced6f4745f529098cae38a5bd3144035a1318c

--
Mattijs


[PATCH] bootstd: Fix a handful of doc typos in bootmeth

2024-06-03 Thread Mattijs Korpershoek
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

2024-06-03 Thread Mattijs Korpershoek
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

2024-06-03 Thread Mattijs Korpershoek
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

2024-05-29 Thread Mattijs Korpershoek
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

2024-05-28 Thread Mattijs Korpershoek
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()

2024-05-28 Thread Mattijs Korpershoek
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

2024-05-28 Thread Mattijs Korpershoek
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

2024-05-28 Thread Mattijs Korpershoek
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

2024-05-28 Thread Mattijs Korpershoek
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

2024-05-24 Thread Mattijs Korpershoek
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

2024-05-23 Thread Mattijs Korpershoek
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

2024-05-23 Thread Mattijs Korpershoek
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

2024-05-23 Thread Mattijs Korpershoek
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

2024-05-23 Thread Mattijs Korpershoek
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

2024-05-22 Thread Mattijs Korpershoek
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+

2024-05-22 Thread Mattijs Korpershoek
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

2024-05-16 Thread Mattijs Korpershoek
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

2024-05-16 Thread Mattijs Korpershoek
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

2024-05-16 Thread Mattijs Korpershoek
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

2024-05-16 Thread Mattijs Korpershoek
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

2024-05-16 Thread Mattijs Korpershoek
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

2024-05-16 Thread Mattijs Korpershoek
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

2024-05-16 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-14 Thread Mattijs Korpershoek
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

2024-05-07 Thread Mattijs Korpershoek
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

2024-05-07 Thread Mattijs Korpershoek
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

2024-05-07 Thread Mattijs Korpershoek
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

2024-05-07 Thread Mattijs Korpershoek
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

2024-05-03 Thread Mattijs Korpershoek
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

2024-05-02 Thread Mattijs Korpershoek
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

2024-05-02 Thread Mattijs Korpershoek
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

2024-05-02 Thread Mattijs Korpershoek
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

2024-05-02 Thread Mattijs Korpershoek
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

2024-05-02 Thread Mattijs Korpershoek
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 
>> 


  1   2   3   4   5   >