[PATCH v2 0/7] remove some usage of gadget_is_{*}speed() API

[PATCH v2 0/7] remove some usage of gadget_is_{*}speed() API

[Linyu Yuan]

This series try to remove some usage of gadget_is_dualspeed(),
gadget_is_superspeed() and gadget_is_superspeed_plus().

please check each change for details.

V2: fix comments from Oliver Neukum <oneukum@suse.com>

Linyu Yuan (7):
  usb: gadget: use working speed to calcaulate network bitrate and qlen
  usb: gadget: add a inline function gether_bitrate()
  usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()
  usb: gadget: unconditionally allocate hs/ss descriptor in bind
    operation
  usb: gadget: config: remove max speed check in
    usb_assign_descriptors()
  usb: gadget: composite: cleanup function config_ep_by_speed_and_alt()
  usb: gadget: remove max support speed info in bind operation

 drivers/usb/gadget/composite.c             | 34 ++++++-------
 drivers/usb/gadget/config.c                |  8 ++--
 drivers/usb/gadget/function/f_acm.c        |  4 +-
 drivers/usb/gadget/function/f_ecm.c        | 19 ++------
 drivers/usb/gadget/function/f_eem.c        |  4 +-
 drivers/usb/gadget/function/f_loopback.c   |  4 +-
 drivers/usb/gadget/function/f_midi.c       | 56 +++++++++-------------
 drivers/usb/gadget/function/f_midi2.c      | 44 +++++++----------
 drivers/usb/gadget/function/f_ncm.c        | 23 ++-------
 drivers/usb/gadget/function/f_obex.c       |  3 +-
 drivers/usb/gadget/function/f_rndis.c      | 19 +-------
 drivers/usb/gadget/function/f_serial.c     |  4 +-
 drivers/usb/gadget/function/f_sourcesink.c |  4 +-
 drivers/usb/gadget/function/f_subset.c     |  4 +-
 drivers/usb/gadget/function/f_uvc.c        | 36 +++++---------
 drivers/usb/gadget/function/u_ether.c      |  5 +-
 drivers/usb/gadget/function/u_ether.h      | 13 +++++
 17 files changed, 99 insertions(+), 185 deletions(-)

-- 
2.17.1

[PATCH v2 1/7] usb: gadget: use working speed to calcaulate network bitrate and qlen

[Linyu Yuan]

Take ecm_bitrate() as example, it will be called after gadget device
link speed negotiation, consider code
if (gadget_is_superspeed(g) && g->speed == USB_SPEED_SUPER),
if a gadget device link speed is USB_SPEED_SUPER,
gadget_is_superspeed(g) must be true, or not it is a wrong
configuration of gadget max support speed.

Remove gadget_is_superspeed(g) checking should be safe, and remove other
similar operation in ncm, rndis, u_ether.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: move this as first patch to fix comment from Oliver Neukum <oneukum@suse.com>

 drivers/usb/gadget/function/f_ecm.c   | 4 ++--
 drivers/usb/gadget/function/f_ncm.c   | 6 +++---
 drivers/usb/gadget/function/f_rndis.c | 6 +++---
 drivers/usb/gadget/function/u_ether.c | 5 ++---
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index c6e63ad77a40..cbe05da94bde 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -68,9 +68,9 @@ static inline struct f_ecm *func_to_ecm(struct usb_function *f)
 /* peak (theoretical) bulk transfer rate in bits-per-second */
 static inline unsigned ecm_bitrate(struct usb_gadget *g)
 {
-	if (gadget_is_superspeed(g) && g->speed == USB_SPEED_SUPER)
+	if (g->speed == USB_SPEED_SUPER)
 		return 13 * 1024 * 8 * 1000 * 8;
-	else if (gadget_is_dualspeed(g) && g->speed == USB_SPEED_HIGH)
+	else if (g->speed == USB_SPEED_HIGH)
 		return 13 * 512 * 8 * 1000 * 8;
 	else
 		return 19 * 64 * 1 * 1000 * 8;
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 424bb3b666db..e6dac5510540 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -85,11 +85,11 @@ static inline unsigned ncm_bitrate(struct usb_gadget *g)
 {
 	if (!g)
 		return 0;
-	else if (gadget_is_superspeed(g) && g->speed >= USB_SPEED_SUPER_PLUS)
+	else if (g->speed >= USB_SPEED_SUPER_PLUS)
 		return 4250000000U;
-	else if (gadget_is_superspeed(g) && g->speed == USB_SPEED_SUPER)
+	else if (g->speed == USB_SPEED_SUPER)
 		return 3750000000U;
-	else if (gadget_is_dualspeed(g) && g->speed == USB_SPEED_HIGH)
+	else if (g->speed == USB_SPEED_HIGH)
 		return 13 * 512 * 8 * 1000 * 8;
 	else
 		return 19 *  64 * 1 * 1000 * 8;
diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index ee95e8f5f9d4..eff5d7cbce00 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -87,11 +87,11 @@ static inline struct f_rndis *func_to_rndis(struct usb_function *f)
 /* peak (theoretical) bulk transfer rate in bits-per-second */
 static unsigned int bitrate(struct usb_gadget *g)
 {
-	if (gadget_is_superspeed(g) && g->speed >= USB_SPEED_SUPER_PLUS)
+	if (g->speed >= USB_SPEED_SUPER_PLUS)
 		return 4250000000U;
-	if (gadget_is_superspeed(g) && g->speed == USB_SPEED_SUPER)
+	if (g->speed == USB_SPEED_SUPER)
 		return 3750000000U;
-	else if (gadget_is_dualspeed(g) && g->speed == USB_SPEED_HIGH)
+	else if (g->speed == USB_SPEED_HIGH)
 		return 13 * 512 * 8 * 1000 * 8;
 	else
 		return 19 * 64 * 1 * 1000 * 8;
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index a366abb45623..4bb0553da658 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -93,11 +93,10 @@ struct eth_dev {
 
 #define DEFAULT_QLEN	2	/* double buffering by default */
 
-/* for dual-speed hardware, use deeper queues at high/super speed */
+/* use deeper queues at high/super speed */
 static inline int qlen(struct usb_gadget *gadget, unsigned qmult)
 {
-	if (gadget_is_dualspeed(gadget) && (gadget->speed == USB_SPEED_HIGH ||
-					    gadget->speed >= USB_SPEED_SUPER))
+	if (gadget->speed == USB_SPEED_HIGH || gadget->speed >= USB_SPEED_SUPER)
 		return qmult * DEFAULT_QLEN;
 	else
 		return DEFAULT_QLEN;
-- 
2.17.1

[PATCH v2 2/7] usb: gadget: add a inline function gether_bitrate()

[Linyu Yuan]

In function ecm_bitrate(), it is not support report bit rate for super
speed plus mode, but it can use same bit rate value defined in ncm and
rndis.

Add a common inline function gether_bitrate() which report different for
all possible speeds, it can be used by ecm, ncm and rndis, also remove
old function from them.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: fix comment from Oliver Neukum <oneukum@suse.com>

 drivers/usb/gadget/function/f_ecm.c   | 15 ++-------------
 drivers/usb/gadget/function/f_ncm.c   | 19 ++-----------------
 drivers/usb/gadget/function/f_rndis.c | 15 +--------------
 drivers/usb/gadget/function/u_ether.h | 13 +++++++++++++
 4 files changed, 18 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index cbe05da94bde..7e943b562348 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -65,17 +65,6 @@ static inline struct f_ecm *func_to_ecm(struct usb_function *f)
 	return container_of(f, struct f_ecm, port.func);
 }
 
-/* peak (theoretical) bulk transfer rate in bits-per-second */
-static inline unsigned ecm_bitrate(struct usb_gadget *g)
-{
-	if (g->speed == USB_SPEED_SUPER)
-		return 13 * 1024 * 8 * 1000 * 8;
-	else if (g->speed == USB_SPEED_HIGH)
-		return 13 * 512 * 8 * 1000 * 8;
-	else
-		return 19 * 64 * 1 * 1000 * 8;
-}
-
 /*-------------------------------------------------------------------------*/
 
 /*
@@ -411,10 +400,10 @@ static void ecm_do_notify(struct f_ecm *ecm)
 
 		/* SPEED_CHANGE data is up/down speeds in bits/sec */
 		data = req->buf + sizeof *event;
-		data[0] = cpu_to_le32(ecm_bitrate(cdev->gadget));
+		data[0] = cpu_to_le32(gether_bitrate(cdev->gadget));
 		data[1] = data[0];
 
-		DBG(cdev, "notify speed %d\n", ecm_bitrate(cdev->gadget));
+		DBG(cdev, "notify speed %d\n", gether_bitrate(cdev->gadget));
 		ecm->notify_state = ECM_NOTIFY_NONE;
 		break;
 	}
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index e6dac5510540..0feadf686a31 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -80,21 +80,6 @@ static inline struct f_ncm *func_to_ncm(struct usb_function *f)
 	return container_of(f, struct f_ncm, port.func);
 }
 
-/* peak (theoretical) bulk transfer rate in bits-per-second */
-static inline unsigned ncm_bitrate(struct usb_gadget *g)
-{
-	if (!g)
-		return 0;
-	else if (g->speed >= USB_SPEED_SUPER_PLUS)
-		return 4250000000U;
-	else if (g->speed == USB_SPEED_SUPER)
-		return 3750000000U;
-	else if (g->speed == USB_SPEED_HIGH)
-		return 13 * 512 * 8 * 1000 * 8;
-	else
-		return 19 *  64 * 1 * 1000 * 8;
-}
-
 /*-------------------------------------------------------------------------*/
 
 /*
@@ -576,10 +561,10 @@ static void ncm_do_notify(struct f_ncm *ncm)
 
 		/* SPEED_CHANGE data is up/down speeds in bits/sec */
 		data = req->buf + sizeof *event;
-		data[0] = cpu_to_le32(ncm_bitrate(cdev->gadget));
+		data[0] = cpu_to_le32(gether_bitrate(cdev->gadget));
 		data[1] = data[0];
 
-		DBG(cdev, "notify speed %u\n", ncm_bitrate(cdev->gadget));
+		DBG(cdev, "notify speed %u\n", gether_bitrate(cdev->gadget));
 		ncm->notify_state = NCM_NOTIFY_CONNECT;
 		break;
 	}
diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index eff5d7cbce00..ed1c3eb91d3b 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -84,19 +84,6 @@ static inline struct f_rndis *func_to_rndis(struct usb_function *f)
 	return container_of(f, struct f_rndis, port.func);
 }
 
-/* peak (theoretical) bulk transfer rate in bits-per-second */
-static unsigned int bitrate(struct usb_gadget *g)
-{
-	if (g->speed >= USB_SPEED_SUPER_PLUS)
-		return 4250000000U;
-	if (g->speed == USB_SPEED_SUPER)
-		return 3750000000U;
-	else if (g->speed == USB_SPEED_HIGH)
-		return 13 * 512 * 8 * 1000 * 8;
-	else
-		return 19 * 64 * 1 * 1000 * 8;
-}
-
 /*-------------------------------------------------------------------------*/
 
 /*
@@ -640,7 +627,7 @@ static void rndis_open(struct gether *geth)
 	DBG(cdev, "%s\n", __func__);
 
 	rndis_set_param_medium(rndis->params, RNDIS_MEDIUM_802_3,
-				bitrate(cdev->gadget) / 100);
+				gether_bitrate(cdev->gadget) / 100);
 	rndis_signal_connect(rndis->params);
 }
 
diff --git a/drivers/usb/gadget/function/u_ether.h b/drivers/usb/gadget/function/u_ether.h
index 851ee10d6e63..34be220cef77 100644
--- a/drivers/usb/gadget/function/u_ether.h
+++ b/drivers/usb/gadget/function/u_ether.h
@@ -279,4 +279,17 @@ static inline bool can_support_ecm(struct usb_gadget *gadget)
 	return true;
 }
 
+/* peak (theoretical) bulk transfer rate in bits-per-second */
+static inline unsigned int gether_bitrate(struct usb_gadget *g)
+{
+	if (g->speed >= USB_SPEED_SUPER_PLUS)
+		return 4250000000U;
+	if (g->speed == USB_SPEED_SUPER)
+		return 3750000000U;
+	else if (g->speed == USB_SPEED_HIGH)
+		return 13 * 512 * 8 * 1000 * 8;
+	else
+		return 19 * 64 * 1 * 1000 * 8;
+}
+
 #endif /* __U_ETHER_H */
-- 
2.17.1

[PATCH v2 3/7] usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()

[Linyu Yuan]

when call uvc_function_bind(), gadget still have no connection speed,
just follow other gadget function, use fs endpoint descriptor to allocate
a video endpoint, remove gadget_is_{super|dual}speed() API call.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: no change

 drivers/usb/gadget/function/f_uvc.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 5e919fb65833..c8e149f8315f 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -719,21 +719,13 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	}
 	uvc->enable_interrupt_ep = opts->enable_interrupt_ep;
 
-	if (gadget_is_superspeed(c->cdev->gadget))
-		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
-					  &uvc_ss_streaming_comp);
-	else if (gadget_is_dualspeed(cdev->gadget))
-		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
-	else
-		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
-
+	ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
 	if (!ep) {
 		uvcg_info(f, "Unable to allocate streaming EP\n");
 		goto error;
 	}
 	uvc->video.ep = ep;
 
-	uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
 	uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
 	uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
 
-- 
2.17.1

[PATCH v2 4/7] usb: gadget: unconditionally allocate hs/ss descriptor in bind operation

[Linyu Yuan]

Take f_midi_bind() for example,  when composite layer call it, it will
allocate hs descriptor by calling gadget_is_dualspeed() API to check
gadget max support speed capability, but most other gadget function didn't
do like this.

To follow other function drivers, it is safe to remove the check which
mean support all possible link speed by default in function driver.

Similar change apply to midi2 and uvc.

Also in midi and midi2, as there is no descriptor difference between
super speed and super speed plus, follow other gadget function drivers,
do not allocate descriptor for super speed plus, composite layer will
handle it properly.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: no change

 drivers/usb/gadget/function/f_midi.c  | 56 +++++++++++----------------
 drivers/usb/gadget/function/f_midi2.c | 44 ++++++++-------------
 drivers/usb/gadget/function/f_uvc.c   | 26 ++++++-------
 3 files changed, 51 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index fddf539008a9..2d02f25f9597 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -1023,40 +1023,30 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
 	if (!f->fs_descriptors)
 		goto fail_f_midi;
 
-	if (gadget_is_dualspeed(c->cdev->gadget)) {
-		bulk_in_desc.wMaxPacketSize = cpu_to_le16(512);
-		bulk_out_desc.wMaxPacketSize = cpu_to_le16(512);
-		f->hs_descriptors = usb_copy_descriptors(midi_function);
-		if (!f->hs_descriptors)
-			goto fail_f_midi;
-	}
+	bulk_in_desc.wMaxPacketSize = cpu_to_le16(512);
+	bulk_out_desc.wMaxPacketSize = cpu_to_le16(512);
+	f->hs_descriptors = usb_copy_descriptors(midi_function);
+	if (!f->hs_descriptors)
+		goto fail_f_midi;
 
-	if (gadget_is_superspeed(c->cdev->gadget)) {
-		bulk_in_desc.wMaxPacketSize = cpu_to_le16(1024);
-		bulk_out_desc.wMaxPacketSize = cpu_to_le16(1024);
-		i = endpoint_descriptor_index;
-		midi_function[i++] = (struct usb_descriptor_header *)
-				     &bulk_out_desc;
-		midi_function[i++] = (struct usb_descriptor_header *)
-				     &bulk_out_ss_comp_desc;
-		midi_function[i++] = (struct usb_descriptor_header *)
-				     &ms_out_desc;
-		midi_function[i++] = (struct usb_descriptor_header *)
-				     &bulk_in_desc;
-		midi_function[i++] = (struct usb_descriptor_header *)
-				     &bulk_in_ss_comp_desc;
-		midi_function[i++] = (struct usb_descriptor_header *)
-				     &ms_in_desc;
-		f->ss_descriptors = usb_copy_descriptors(midi_function);
-		if (!f->ss_descriptors)
-			goto fail_f_midi;
-
-		if (gadget_is_superspeed_plus(c->cdev->gadget)) {
-			f->ssp_descriptors = usb_copy_descriptors(midi_function);
-			if (!f->ssp_descriptors)
-				goto fail_f_midi;
-		}
-	}
+	bulk_in_desc.wMaxPacketSize = cpu_to_le16(1024);
+	bulk_out_desc.wMaxPacketSize = cpu_to_le16(1024);
+	i = endpoint_descriptor_index;
+	midi_function[i++] = (struct usb_descriptor_header *)
+			     &bulk_out_desc;
+	midi_function[i++] = (struct usb_descriptor_header *)
+			     &bulk_out_ss_comp_desc;
+	midi_function[i++] = (struct usb_descriptor_header *)
+			     &ms_out_desc;
+	midi_function[i++] = (struct usb_descriptor_header *)
+			     &bulk_in_desc;
+	midi_function[i++] = (struct usb_descriptor_header *)
+			     &bulk_in_ss_comp_desc;
+	midi_function[i++] = (struct usb_descriptor_header *)
+			     &ms_in_desc;
+	f->ss_descriptors = usb_copy_descriptors(midi_function);
+	if (!f->ss_descriptors)
+		goto fail_f_midi;
 
 	kfree(midi_function);
 
diff --git a/drivers/usb/gadget/function/f_midi2.c b/drivers/usb/gadget/function/f_midi2.c
index 5a971ba600fe..ec8cd7c7bbfc 100644
--- a/drivers/usb/gadget/function/f_midi2.c
+++ b/drivers/usb/gadget/function/f_midi2.c
@@ -1731,7 +1731,6 @@ static int f_midi2_create_usb_configs(struct f_midi2 *midi2,
 		midi1_out_eps = midi2_midi1_ep_out_descs;
 		break;
 	case USB_SPEED_SUPER:
-	case USB_SPEED_SUPER_PLUS:
 		midi2_midi1_ep_out_desc.wMaxPacketSize = cpu_to_le16(1024);
 		midi2_midi1_ep_in_desc.wMaxPacketSize = cpu_to_le16(1024);
 		for (i = 0; i < midi2->num_eps; i++)
@@ -2001,36 +2000,25 @@ static int f_midi2_bind(struct usb_configuration *c, struct usb_function *f)
 	}
 	f_midi2_free_usb_configs(&config);
 
-	if (gadget_is_dualspeed(midi2->gadget)) {
-		status = f_midi2_create_usb_configs(midi2, &config, USB_SPEED_HIGH);
-		if (status < 0)
-			goto fail;
-		f->hs_descriptors = usb_copy_descriptors(config.list);
-		if (!f->hs_descriptors) {
-			status = -ENOMEM;
-			goto fail;
-		}
-		f_midi2_free_usb_configs(&config);
+	status = f_midi2_create_usb_configs(midi2, &config, USB_SPEED_HIGH);
+	if (status < 0)
+		goto fail;
+	f->hs_descriptors = usb_copy_descriptors(config.list);
+	if (!f->hs_descriptors) {
+		status = -ENOMEM;
+		goto fail;
 	}
+	f_midi2_free_usb_configs(&config);
 
-	if (gadget_is_superspeed(midi2->gadget)) {
-		status = f_midi2_create_usb_configs(midi2, &config, USB_SPEED_SUPER);
-		if (status < 0)
-			goto fail;
-		f->ss_descriptors = usb_copy_descriptors(config.list);
-		if (!f->ss_descriptors) {
-			status = -ENOMEM;
-			goto fail;
-		}
-		if (gadget_is_superspeed_plus(midi2->gadget)) {
-			f->ssp_descriptors = usb_copy_descriptors(config.list);
-			if (!f->ssp_descriptors) {
-				status = -ENOMEM;
-				goto fail;
-			}
-		}
-		f_midi2_free_usb_configs(&config);
+	status = f_midi2_create_usb_configs(midi2, &config, USB_SPEED_SUPER);
+	if (status < 0)
+		goto fail;
+	f->ss_descriptors = usb_copy_descriptors(config.list);
+	if (!f->ss_descriptors) {
+		status = -ENOMEM;
+		goto fail;
 	}
+	f_midi2_free_usb_configs(&config);
 
 	mutex_unlock(&f_midi2_desc_mutex);
 	return 0;
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index c8e149f8315f..faa398109431 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -780,21 +780,19 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		f->fs_descriptors = NULL;
 		goto error;
 	}
-	if (gadget_is_dualspeed(cdev->gadget)) {
-		f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
-		if (IS_ERR(f->hs_descriptors)) {
-			ret = PTR_ERR(f->hs_descriptors);
-			f->hs_descriptors = NULL;
-			goto error;
-		}
+
+	f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
+	if (IS_ERR(f->hs_descriptors)) {
+		ret = PTR_ERR(f->hs_descriptors);
+		f->hs_descriptors = NULL;
+		goto error;
 	}
-	if (gadget_is_superspeed(c->cdev->gadget)) {
-		f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER);
-		if (IS_ERR(f->ss_descriptors)) {
-			ret = PTR_ERR(f->ss_descriptors);
-			f->ss_descriptors = NULL;
-			goto error;
-		}
+
+	f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER);
+	if (IS_ERR(f->ss_descriptors)) {
+		ret = PTR_ERR(f->ss_descriptors);
+		f->ss_descriptors = NULL;
+		goto error;
 	}
 
 	/* Preallocate control endpoint request. */
-- 
2.17.1

[PATCH v2 5/7] usb: gadget: config: remove max speed check in usb_assign_descriptors()

[Linyu Yuan]

usb_assign_descriptors() usally called inside function bind operation,
and gadget still have no working connection speed, let's support all
speed at this point, it may possible allocate extra memory to store
descriptors, but it is small and acceptable.

Remove gadget_is_{*}speed() API checking to allow support all speed.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: no change

 drivers/usb/gadget/config.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/config.c b/drivers/usb/gadget/config.c
index 05507606b2b4..b1f625245713 100644
--- a/drivers/usb/gadget/config.c
+++ b/drivers/usb/gadget/config.c
@@ -162,8 +162,6 @@ int usb_assign_descriptors(struct usb_function *f,
 		struct usb_descriptor_header **ss,
 		struct usb_descriptor_header **ssp)
 {
-	struct usb_gadget *g = f->config->cdev->gadget;
-
 	/* super-speed-plus descriptor falls back to super-speed one,
 	 * if such a descriptor was provided, thus avoiding a NULL
 	 * pointer dereference if a 5gbps capable gadget is used with
@@ -177,17 +175,17 @@ int usb_assign_descriptors(struct usb_function *f,
 		if (!f->fs_descriptors)
 			goto err;
 	}
-	if (hs && gadget_is_dualspeed(g)) {
+	if (hs) {
 		f->hs_descriptors = usb_copy_descriptors(hs);
 		if (!f->hs_descriptors)
 			goto err;
 	}
-	if (ss && gadget_is_superspeed(g)) {
+	if (ss) {
 		f->ss_descriptors = usb_copy_descriptors(ss);
 		if (!f->ss_descriptors)
 			goto err;
 	}
-	if (ssp && gadget_is_superspeed_plus(g)) {
+	if (ssp) {
 		f->ssp_descriptors = usb_copy_descriptors(ssp);
 		if (!f->ssp_descriptors)
 			goto err;
-- 
2.17.1

[PATCH v2 6/7] usb: gadget: composite: cleanup function config_ep_by_speed_and_alt()

[Linyu Yuan]

When call this function, gadget already have working speed, if it is
USB_SPEED_SUPER_PLUS, in theroy gadget_is_superspeed_plus() checking
should be true, so there is no need to call it. it is same for other
working speed.

Remove all gadget_is_{*}speed_plus() API call to clean it up.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: no change

 drivers/usb/gadget/composite.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index dd9b90481b4c..0ace45b66a31 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -170,33 +170,27 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
 	/* select desired speed */
 	switch (g->speed) {
 	case USB_SPEED_SUPER_PLUS:
-		if (gadget_is_superspeed_plus(g)) {
-			if (f->ssp_descriptors) {
-				speed_desc = f->ssp_descriptors;
-				want_comp_desc = 1;
-				break;
-			}
-			incomplete_desc = true;
+		if (f->ssp_descriptors) {
+			speed_desc = f->ssp_descriptors;
+			want_comp_desc = 1;
+			break;
 		}
+		incomplete_desc = true;
 		fallthrough;
 	case USB_SPEED_SUPER:
-		if (gadget_is_superspeed(g)) {
-			if (f->ss_descriptors) {
-				speed_desc = f->ss_descriptors;
-				want_comp_desc = 1;
-				break;
-			}
-			incomplete_desc = true;
+		if (f->ss_descriptors) {
+			speed_desc = f->ss_descriptors;
+			want_comp_desc = 1;
+			break;
 		}
+		incomplete_desc = true;
 		fallthrough;
 	case USB_SPEED_HIGH:
-		if (gadget_is_dualspeed(g)) {
-			if (f->hs_descriptors) {
-				speed_desc = f->hs_descriptors;
-				break;
-			}
-			incomplete_desc = true;
+		if (f->hs_descriptors) {
+			speed_desc = f->hs_descriptors;
+			break;
 		}
+		incomplete_desc = true;
 		fallthrough;
 	default:
 		speed_desc = f->fs_descriptors;
-- 
2.17.1

[PATCH v2 7/7] usb: gadget: remove max support speed info in bind operation

[Linyu Yuan]

Take ecm_bind() for example, it call gadget_is_{*}speed() API to show
gadget max support speed, it is not much help, remove the API usage here
is safe.

Similar change apply to acm,eem,loopback,ncm,obex,rndis,serial,
sourcesink,subset functions.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: no change

 drivers/usb/gadget/function/f_acm.c        | 4 +---
 drivers/usb/gadget/function/f_ecm.c        | 4 +---
 drivers/usb/gadget/function/f_eem.c        | 4 +---
 drivers/usb/gadget/function/f_loopback.c   | 4 +---
 drivers/usb/gadget/function/f_ncm.c        | 4 +---
 drivers/usb/gadget/function/f_obex.c       | 3 +--
 drivers/usb/gadget/function/f_rndis.c      | 4 +---
 drivers/usb/gadget/function/f_serial.c     | 4 +---
 drivers/usb/gadget/function/f_sourcesink.c | 4 +---
 drivers/usb/gadget/function/f_subset.c     | 4 +---
 10 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index cb523f118f04..f616059c5e1e 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -691,10 +691,8 @@ acm_bind(struct usb_configuration *c, struct usb_function *f)
 		goto fail;
 
 	dev_dbg(&cdev->gadget->dev,
-		"acm ttyGS%d: %s speed IN/%s OUT/%s NOTIFY/%s\n",
+		"acm ttyGS%d: IN/%s OUT/%s NOTIFY/%s\n",
 		acm->port_num,
-		gadget_is_superspeed(c->cdev->gadget) ? "super" :
-		gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
 		acm->port.in->name, acm->port.out->name,
 		acm->notify->name);
 	return 0;
diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index 7e943b562348..f55f60639e42 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -788,9 +788,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 	ecm->port.open = ecm_open;
 	ecm->port.close = ecm_close;
 
-	DBG(cdev, "CDC Ethernet: %s speed IN/%s OUT/%s NOTIFY/%s\n",
-			gadget_is_superspeed(c->cdev->gadget) ? "super" :
-			gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
+	DBG(cdev, "CDC Ethernet: IN/%s OUT/%s NOTIFY/%s\n",
 			ecm->port.in_ep->name, ecm->port.out_ep->name,
 			ecm->notify->name);
 	return 0;
diff --git a/drivers/usb/gadget/function/f_eem.c b/drivers/usb/gadget/function/f_eem.c
index 5d38f29bda72..3b445bd88498 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -311,9 +311,7 @@ static int eem_bind(struct usb_configuration *c, struct usb_function *f)
 	if (status)
 		goto fail;
 
-	DBG(cdev, "CDC Ethernet (EEM): %s speed IN/%s OUT/%s\n",
-			gadget_is_superspeed(c->cdev->gadget) ? "super" :
-			gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
+	DBG(cdev, "CDC Ethernet (EEM): IN/%s OUT/%s\n",
 			eem->port.in_ep->name, eem->port.out_ep->name);
 	return 0;
 
diff --git a/drivers/usb/gadget/function/f_loopback.c b/drivers/usb/gadget/function/f_loopback.c
index ae41f556eb75..17ac6ace0cff 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -211,9 +211,7 @@ static int loopback_bind(struct usb_configuration *c, struct usb_function *f)
 	if (ret)
 		return ret;
 
-	DBG(cdev, "%s speed %s: IN/%s, OUT/%s\n",
-	    (gadget_is_superspeed(c->cdev->gadget) ? "super" :
-	     (gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full")),
+	DBG(cdev, "%s: IN/%s, OUT/%s\n",
 			f->name, loop->in_ep->name, loop->out_ep->name);
 	return 0;
 }
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 0feadf686a31..feccf4c8cc4f 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1529,9 +1529,7 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
 	hrtimer_init(&ncm->task_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
 	ncm->task_timer.function = ncm_tx_timeout;
 
-	DBG(cdev, "CDC Network: %s speed IN/%s OUT/%s NOTIFY/%s\n",
-			gadget_is_superspeed(c->cdev->gadget) ? "super" :
-			gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
+	DBG(cdev, "CDC Network: IN/%s OUT/%s NOTIFY/%s\n",
 			ncm->port.in_ep->name, ncm->port.out_ep->name,
 			ncm->notify->name);
 	return 0;
diff --git a/drivers/usb/gadget/function/f_obex.c b/drivers/usb/gadget/function/f_obex.c
index ab26d84ed95e..dcb093210305 100644
--- a/drivers/usb/gadget/function/f_obex.c
+++ b/drivers/usb/gadget/function/f_obex.c
@@ -365,9 +365,8 @@ static int obex_bind(struct usb_configuration *c, struct usb_function *f)
 	if (status)
 		goto fail;
 
-	dev_dbg(&cdev->gadget->dev, "obex ttyGS%d: %s speed IN/%s OUT/%s\n",
+	dev_dbg(&cdev->gadget->dev, "obex ttyGS%d: IN/%s OUT/%s\n",
 		obex->port_num,
-		gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
 		obex->port.in->name, obex->port.out->name);
 
 	return 0;
diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index ed1c3eb91d3b..b47f99d17ee9 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -798,9 +798,7 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
 	 * until we're activated via set_alt().
 	 */
 
-	DBG(cdev, "RNDIS: %s speed IN/%s OUT/%s NOTIFY/%s\n",
-			gadget_is_superspeed(c->cdev->gadget) ? "super" :
-			gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
+	DBG(cdev, "RNDIS: IN/%s OUT/%s NOTIFY/%s\n",
 			rndis->port.in_ep->name, rndis->port.out_ep->name,
 			rndis->notify->name);
 	return 0;
diff --git a/drivers/usb/gadget/function/f_serial.c b/drivers/usb/gadget/function/f_serial.c
index a9480b9e312e..65c50092aea2 100644
--- a/drivers/usb/gadget/function/f_serial.c
+++ b/drivers/usb/gadget/function/f_serial.c
@@ -236,10 +236,8 @@ static int gser_bind(struct usb_configuration *c, struct usb_function *f)
 			gser_ss_function, gser_ss_function);
 	if (status)
 		goto fail;
-	dev_dbg(&cdev->gadget->dev, "generic ttyGS%d: %s speed IN/%s OUT/%s\n",
+	dev_dbg(&cdev->gadget->dev, "generic ttyGS%d: IN/%s OUT/%s\n",
 		gser->port_num,
-		gadget_is_superspeed(c->cdev->gadget) ? "super" :
-		gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
 		gser->port.in->name, gser->port.out->name);
 	return 0;
 
diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
index 6803cd60cc6d..2edbd9b510d6 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -436,9 +436,7 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
 	if (ret)
 		return ret;
 
-	DBG(cdev, "%s speed %s: IN/%s, OUT/%s, ISO-IN/%s, ISO-OUT/%s\n",
-	    (gadget_is_superspeed(c->cdev->gadget) ? "super" :
-	     (gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full")),
+	DBG(cdev, "%s: IN/%s, OUT/%s, ISO-IN/%s, ISO-OUT/%s\n",
 			f->name, ss->in_ep->name, ss->out_ep->name,
 			ss->iso_in_ep ? ss->iso_in_ep->name : "<none>",
 			ss->iso_out_ep ? ss->iso_out_ep->name : "<none>");
diff --git a/drivers/usb/gadget/function/f_subset.c b/drivers/usb/gadget/function/f_subset.c
index 51c1cae162d9..8ae9689ef2a0 100644
--- a/drivers/usb/gadget/function/f_subset.c
+++ b/drivers/usb/gadget/function/f_subset.c
@@ -367,9 +367,7 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
 	 * until we're activated via set_alt().
 	 */
 
-	DBG(cdev, "CDC Subset: %s speed IN/%s OUT/%s\n",
-			gadget_is_superspeed(c->cdev->gadget) ? "super" :
-			gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
+	DBG(cdev, "CDC Subset: IN/%s OUT/%s\n",
 			geth->port.in_ep->name, geth->port.out_ep->name);
 	return 0;
 
-- 
2.17.1

Re: [PATCH v2 3/7] usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()

[Frank Li]

On Thu, Aug 03, 2023 at 05:10:49PM +0800, Linyu Yuan wrote:
> when call uvc_function_bind(), gadget still have no connection speed,
> just follow other gadget function, use fs endpoint descriptor to allocate
> a video endpoint, remove gadget_is_{super|dual}speed() API call.
> 
> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
> ---
> v2: no change
> 
>  drivers/usb/gadget/function/f_uvc.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index 5e919fb65833..c8e149f8315f 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -719,21 +719,13 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
>  	}
>  	uvc->enable_interrupt_ep = opts->enable_interrupt_ep;
>  
> -	if (gadget_is_superspeed(c->cdev->gadget))
> -		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
> -					  &uvc_ss_streaming_comp);
> -	else if (gadget_is_dualspeed(cdev->gadget))
> -		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
> -	else
> -		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
> -
> +	ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);

Some UDC driver use gadget_check_config() and match_ep() to allocate EP
internal fifo memory resource, if only pass download full speed EP.

UDC will allocate too much internal memory to each EP. It may failure when
use ss config. Generally, ss config have bigger max package size.

Frank

>  	if (!ep) {
>  		uvcg_info(f, "Unable to allocate streaming EP\n");
>  		goto error;
>  	}
>  	uvc->video.ep = ep;
>  
> -	uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
>  	uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
>  	uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
>  
> -- 
> 2.17.1
> 

Re: [PATCH v2 3/7] usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()

[yuan linyu]

On 2023/12/20 00:17, Frank Li wrote:
> On Thu, Aug 03, 2023 at 05:10:49PM +0800, Linyu Yuan wrote:
>> when call uvc_function_bind(), gadget still have no connection speed,
>> just follow other gadget function, use fs endpoint descriptor to allocate
>> a video endpoint, remove gadget_is_{super|dual}speed() API call.
>>
>> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
>> ---
>> v2: no change
>>
>>  drivers/usb/gadget/function/f_uvc.c | 10 +---------
>>  1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
>> index 5e919fb65833..c8e149f8315f 100644
>> --- a/drivers/usb/gadget/function/f_uvc.c
>> +++ b/drivers/usb/gadget/function/f_uvc.c
>> @@ -719,21 +719,13 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
>>  	}
>>  	uvc->enable_interrupt_ep = opts->enable_interrupt_ep;
>>  
>> -	if (gadget_is_superspeed(c->cdev->gadget))
>> -		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
>> -					  &uvc_ss_streaming_comp);
>> -	else if (gadget_is_dualspeed(cdev->gadget))
>> -		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
>> -	else
>> -		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
>> -
>> +	ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
> Some UDC driver use gadget_check_config() and match_ep() to allocate EP
> internal fifo memory resource, if only pass download full speed EP.
Could you share  the detail of problem ? do you mean find another different endpoint compared

with change before?


From my understanding, according to configfs gadget driver design, when find a endpoint, there is no

working speed, this means each hardware endpoint should support all possible speeds.
>
> UDC will allocate too much internal memory to each EP. It may failure when
> use ss config. Generally, ss config have bigger max package size.
is there another way to solve your issue in your driver ?
>
> Frank
>
>>  	if (!ep) {
>>  		uvcg_info(f, "Unable to allocate streaming EP\n");
>>  		goto error;
>>  	}
>>  	uvc->video.ep = ep;
>>  
>> -	uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
>>  	uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
>>  	uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
>>  
>> -- 
>> 2.17.1
>>

Re: [PATCH v2 3/7] usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()

[Frank Li]

On Wed, Dec 20, 2023 at 10:33:17PM +0800, yuan linyu wrote:
> 
> On 2023/12/20 00:17, Frank Li wrote:
> > On Thu, Aug 03, 2023 at 05:10:49PM +0800, Linyu Yuan wrote:
> >> when call uvc_function_bind(), gadget still have no connection speed,
> >> just follow other gadget function, use fs endpoint descriptor to allocate
> >> a video endpoint, remove gadget_is_{super|dual}speed() API call.
> >>
> >> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
> >> ---
> >> v2: no change
> >>
> >>  drivers/usb/gadget/function/f_uvc.c | 10 +---------
> >>  1 file changed, 1 insertion(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> >> index 5e919fb65833..c8e149f8315f 100644
> >> --- a/drivers/usb/gadget/function/f_uvc.c
> >> +++ b/drivers/usb/gadget/function/f_uvc.c
> >> @@ -719,21 +719,13 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
> >>  	}
> >>  	uvc->enable_interrupt_ep = opts->enable_interrupt_ep;
> >>  
> >> -	if (gadget_is_superspeed(c->cdev->gadget))
> >> -		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
> >> -					  &uvc_ss_streaming_comp);
> >> -	else if (gadget_is_dualspeed(cdev->gadget))
> >> -		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
> >> -	else
> >> -		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
> >> -
> >> +	ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
> > Some UDC driver use gadget_check_config() and match_ep() to allocate EP
> > internal fifo memory resource, if only pass download full speed EP.
> Could you share  the detail of problem ? do you mean find another different endpoint compared

The problem is little bit complex. I try to use simple words.

The background:

Generally, UDC have some EP<0..15> and have some internal memory as FIFO.
for example 16K.  You can simple assign EP<n> to 1K memory, which can hold
whole package.

But for UVC, some controller required internal FIFO hold whole frame data

(mult+1) * (MaxBurst +1) * wPackageSize.

For most case,  not every gadget use all 16 EPs. So you can assgin more
memory into one EP, so it will reduce bus 'ping' package number and reduce
NACK to improve transfer speed.

The problem:
pass fs_stream to udc driver, udc driver's check_config function will see
mult and maxburst is 0. so only reserve 1K for ISO EP, but when try to 
enable EP,  mult is 2, so there are not enough internal memory for it
because more memory already assign to other EPs.

Ideally, when gadget frame work can call check_config again when know
usb speed, but it is not easy to fix it.

Simple method use ss_stream_ep here and other function drviers. Super
speed's package size is bigger than high/full speed. If resource can
support super speed, it can support high/full speed.


/**
 * gadget_is_superspeed() - return true if the hardware handles superspeed
 * @g: controller that might support superspeed
 */

@max_speed: Highest speed the driver handles

And according to gadget_is_superspeed() define, it indicate if udc
controller support supersped, not link speed. 

Orignial code is correct. If UDC support superspeed, then use ss_stream_ep.

becasue superspeed is worse case compared as high and full speed.

So I think original is correct.

Frank.

> 
> with change before?
> 
> 
> >From my understanding, according to configfs gadget driver design, when find a endpoint, there is no
> 
> working speed, this means each hardware endpoint should support all possible speeds.
> >
> > UDC will allocate too much internal memory to each EP. It may failure when
> > use ss config. Generally, ss config have bigger max package size.
> is there another way to solve your issue in your driver ?


> >
> > Frank
> >
> >>  	if (!ep) {
> >>  		uvcg_info(f, "Unable to allocate streaming EP\n");
> >>  		goto error;
> >>  	}
> >>  	uvc->video.ep = ep;
> >>  
> >> -	uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
> >>  	uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
> >>  	uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
> >>  
> >> -- 
> >> 2.17.1
> >>
> 

Re: [PATCH v2 3/7] usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()

[yuan linyu]

On 2023/12/21 00:02, Frank Li wrote:
>>> Some UDC driver use gadget_check_config() and match_ep() to allocate EP
>>> internal fifo memory resource, if only pass download full speed EP.
>> Could you share  the detail of problem ? do you mean find another different endpoint compared
> The problem is little bit complex. I try to use simple words.
>
> The background:
>
> Generally, UDC have some EP<0..15> and have some internal memory as FIFO.
> for example 16K.  You can simple assign EP<n> to 1K memory, which can hold
> whole package.
>
> But for UVC, some controller required internal FIFO hold whole frame data
>
> (mult+1) * (MaxBurst +1) * wPackageSize.
>
> For most case,  not every gadget use all 16 EPs. So you can assgin more
> memory into one EP, so it will reduce bus 'ping' package number and reduce
> NACK to improve transfer speed.
>
> The problem:
> pass fs_stream to udc driver, udc driver's check_config function will see
> mult and maxburst is 0. so only reserve 1K for ISO EP, but when try to 
> enable EP,  mult is 2, so there are not enough internal memory for it
> because more memory already assign to other EPs.
>
> Ideally, when gadget frame work can call check_config again when know
> usb speed, but it is not easy to fix it.
>
> Simple method use ss_stream_ep here and other function drviers. Super
> speed's package size is bigger than high/full speed. If resource can
> support super speed, it can support high/full speed.


I don't find any difference of uvc_ss_streaming_ep, uvc_hs_streaming_ep, uvc_fs_streaming_ep

descriptors. how difference happen in UDC ?


>
>
> /**
>  * gadget_is_superspeed() - return true if the hardware handles superspeed
>  * @g: controller that might support superspeed
>  */
>
> @max_speed: Highest speed the driver handles
>
> And according to gadget_is_superspeed() define, it indicate if udc
> controller support supersped, not link speed. 
>
> Orignial code is correct. If UDC support superspeed, then use ss_stream_ep.
>
> becasue superspeed is worse case compared as high and full speed.
>
> So I think original is correct.
>
> Frank.
>
>

Re: [PATCH v2 3/7] usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()

[Frank Li]

On Thu, Dec 21, 2023 at 10:14:16PM +0800, yuan linyu wrote:
> 
> On 2023/12/21 00:02, Frank Li wrote:
> >>> Some UDC driver use gadget_check_config() and match_ep() to allocate EP
> >>> internal fifo memory resource, if only pass download full speed EP.
> >> Could you share  the detail of problem ? do you mean find another different endpoint compared
> > The problem is little bit complex. I try to use simple words.
> >
> > The background:
> >
> > Generally, UDC have some EP<0..15> and have some internal memory as FIFO.
> > for example 16K.  You can simple assign EP<n> to 1K memory, which can hold
> > whole package.
> >
> > But for UVC, some controller required internal FIFO hold whole frame data
> >
> > (mult+1) * (MaxBurst +1) * wPackageSize.
> >
> > For most case,  not every gadget use all 16 EPs. So you can assgin more
> > memory into one EP, so it will reduce bus 'ping' package number and reduce
> > NACK to improve transfer speed.
> >
> > The problem:
> > pass fs_stream to udc driver, udc driver's check_config function will see
> > mult and maxburst is 0. so only reserve 1K for ISO EP, but when try to 
> > enable EP,  mult is 2, so there are not enough internal memory for it
> > because more memory already assign to other EPs.
> >
> > Ideally, when gadget frame work can call check_config again when know
> > usb speed, but it is not easy to fix it.
> >
> > Simple method use ss_stream_ep here and other function drviers. Super
> > speed's package size is bigger than high/full speed. If resource can
> > support super speed, it can support high/full speed.
> 
> 
> I don't find any difference of uvc_ss_streaming_ep, uvc_hs_streaming_ep, uvc_fs_streaming_ep
> 
> descriptors. how difference happen in UDC ?

	uvc_hs_streaming_ep.wMaxPacketSize =                                                        
                cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11));

Hight speed will use bit [12:11] as mult

	uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;                                   
        uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;

ss will pass down uvc_ss_streaming_comp descriptor, which have bMaxBurst
and mult information.


Frank

> 
> 
> >
> >
> > /**
> >  * gadget_is_superspeed() - return true if the hardware handles superspeed
> >  * @g: controller that might support superspeed
> >  */
> >
> > @max_speed: Highest speed the driver handles
> >
> > And according to gadget_is_superspeed() define, it indicate if udc
> > controller support supersped, not link speed. 
> >
> > Orignial code is correct. If UDC support superspeed, then use ss_stream_ep.
> >
> > becasue superspeed is worse case compared as high and full speed.
> >
> > So I think original is correct.
> >
> > Frank.
> >
> >
>