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

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Marek,

Thank you for the patch.

On dim., juin 09, 2024 at 23:32, Marek Vasut <marek.vasut+renesas@mailbox.org> 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 <marek.vasut+renesas@mailbox.org>

Compared with linux commit, and looks good to me.

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

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

Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # vim3

> ---
> Cc: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Cc: Tom Rini <trini@konsulko.com>
> 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 <usb_mass_storage.h>
>  #include <dfu.h>
>  #include <thor.h>
> +#include <version.h>
>  
>  #include <env_callback.h>
>  
> -#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