diff mbox series

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

Message ID 20240609213449.194762-1-marek.vasut+renesas@mailbox.org
State Accepted
Commit 0fca00114a805ec3c68c6156bc4ae6c4214a6e8a
Delegated to: Mattijs Korpershoek
Headers show
Series [1/6] usb: gadget: g_dnl: Drop usb_gadget_controller_number() | expand

Commit Message

Marek Vasut June 9, 2024, 9:32 p.m. UTC
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>
---
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(-)

Comments

Mattijs Korpershoek June 11, 2024, 7:14 a.m. UTC | #1
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
Mattijs Korpershoek July 5, 2024, 12:02 p.m. UTC | #2
Hi,

On Sun, 09 Jun 2024 23:32:14 +0200, 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.
> 
> [...]

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

[1/6] usb: gadget: g_dnl: Drop usb_gadget_controller_number()
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/0fca00114a805ec3c68c6156bc4ae6c4214a6e8a
[2/6] usb: gadget: ether: Drop usb_gadget_controller_number()
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/7f1b062ca4f23ea77541006759c1ec1108bdda74
[3/6] usb: gadget: Drop usb_gadget_controller_number()
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/2cee3bc6abc592c403140b8a7191879ce95be992
[4/6] usb: gadget: Drop all gadget_is_*() functions
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/db62b6a0a016b69cf0e26eea4004c3edbebd4394
[5/6] usb: gadget: Add full ep_matches() check past .match_ep() callback
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/bd7ec7b04f877b8b4a88d4367f100dc3f0af27a3
[6/6] usb: dwc3: gadget: Convert epautoconf workaround to match_ep callback
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/1918b8010c321c939fdedd6e461ccac87e0d3415

--
Mattijs
diff mbox series

Patch

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 {