[v10,1/6] usb: gadget: udc: core: Introduce check_config to verify USB configuration
diff mbox series

Message ID 1623923899-16759-2-git-send-email-wcheng@codeaurora.org
State New, archived
Headers show
Series
  • Re-introduce TX FIFO resize for larger EP bursting
Related show

Commit Message

Wesley Cheng June 17, 2021, 9:58 a.m. UTC
Some UDCs may have constraints on how many high bandwidth endpoints it can
support in a certain configuration.  This API allows for the composite
driver to pass down the total number of endpoints to the UDC so it can verify
it has the required resources to support the configuration.

Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
---
 drivers/usb/gadget/udc/core.c | 25 +++++++++++++++++++++++++
 include/linux/usb/gadget.h    |  5 +++++
 2 files changed, 30 insertions(+)

Comments

Greg Kroah-Hartman June 17, 2021, 11:09 a.m. UTC | #1
On Thu, Jun 17, 2021 at 02:58:14AM -0700, Wesley Cheng wrote:
> Some UDCs may have constraints on how many high bandwidth endpoints it can
> support in a certain configuration.  This API allows for the composite
> driver to pass down the total number of endpoints to the UDC so it can verify
> it has the required resources to support the configuration.
> 
> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
> ---
>  drivers/usb/gadget/udc/core.c | 25 +++++++++++++++++++++++++
>  include/linux/usb/gadget.h    |  5 +++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index b7f0b1e..e33ae2d 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -1003,6 +1003,31 @@ int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
>  }
>  EXPORT_SYMBOL_GPL(usb_gadget_ep_match_desc);
>  
> +/**
> + * usb_gadget_check_config - checks if the UDC can support the number of eps
> + * @gadget: controller to check the USB configuration
> + * @ep_map: bitmap of endpoints being requested by a USB configuration

Will a u64 really hold all of the possible endpoints?

Why make it odd like this, why not just provide a list like we do in the
USB core with the structure that USB drivers use?  What can a driver do
with a bitmap only?


thanks,

greg k-h
Wesley Cheng June 22, 2021, 5:27 a.m. UTC | #2
On 6/17/2021 4:09 AM, Greg KH wrote:
> On Thu, Jun 17, 2021 at 02:58:14AM -0700, Wesley Cheng wrote:
>> Some UDCs may have constraints on how many high bandwidth endpoints it can
>> support in a certain configuration.  This API allows for the composite
>> driver to pass down the total number of endpoints to the UDC so it can verify
>> it has the required resources to support the configuration.
>>
>> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
>> ---
>>  drivers/usb/gadget/udc/core.c | 25 +++++++++++++++++++++++++
>>  include/linux/usb/gadget.h    |  5 +++++
>>  2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
>> index b7f0b1e..e33ae2d 100644
>> --- a/drivers/usb/gadget/udc/core.c
>> +++ b/drivers/usb/gadget/udc/core.c
>> @@ -1003,6 +1003,31 @@ int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
>>  }
>>  EXPORT_SYMBOL_GPL(usb_gadget_ep_match_desc);
>>  
>> +/**
>> + * usb_gadget_check_config - checks if the UDC can support the number of eps
>> + * @gadget: controller to check the USB configuration
>> + * @ep_map: bitmap of endpoints being requested by a USB configuration

Hi Greg,

> 
> Will a u64 really hold all of the possible endpoints?
> 

Ah, should be a u32 bitmap, and that is enough as USB spec only allows
32 EPs (IN+OUT eps) total.  That hasn't changed since USB2, so I assume
that will stay the same moving forward.  I can fix that.

> Why make it odd like this, why not just provide a list like we do in the
> USB core with the structure that USB drivers use?  What can a driver do
> with a bitmap only?
> 

I didn't want the ep bookeeping here to be too complicated.  For
example, in the TXFIFO resize situation, just knowing the number of
endpoints used can help determine if we have enough internal controller
memory to allocate per endpoint. (at minimum 1 FIFO per EP)  If the USB
configuration is going to be requesting more endpoints than FIFO memory
it has (unlikely), then it will fail the config bind.  Otherwise, we'd
end up enumerating w/ the host w/ the interfaces that were starved of
FIFO memory to be broken/non-functional.  This was one of the concerns
that Felipe had in our initial discussions.

In addition, at the time of function driver binding, the amount of
information we have is minimal per endpoint, as most of it is populated
once the host places the device into the CONFIGURED state. (ie when the
device knows which configuration is being enabled, hence which EPs are
being used)

Thanks
Wesley Cheng

> 
> thanks,
> 
> greg k-h
>

Patch
diff mbox series

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index b7f0b1e..e33ae2d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1003,6 +1003,31 @@  int usb_gadget_ep_match_desc(struct usb_gadget *gadget,
 }
 EXPORT_SYMBOL_GPL(usb_gadget_ep_match_desc);
 
+/**
+ * usb_gadget_check_config - checks if the UDC can support the number of eps
+ * @gadget: controller to check the USB configuration
+ * @ep_map: bitmap of endpoints being requested by a USB configuration
+ *
+ * Ensure that a UDC is able to support the number of endpoints within a USB
+ * configuration, and that there are no resource limitations to support all
+ * requested eps.
+ *
+ * Returns zero on success, else a negative errno.
+ */
+int usb_gadget_check_config(struct usb_gadget *gadget, unsigned long ep_map)
+{
+	int ret = 0;
+
+	if (!gadget->ops->check_config)
+		goto out;
+
+	ret = gadget->ops->check_config(gadget, ep_map);
+
+out:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_check_config);
+
 /* ------------------------------------------------------------------------- */
 
 static void usb_gadget_state_work(struct work_struct *work)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 75c7538..fcde3b3 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -329,6 +329,7 @@  struct usb_gadget_ops {
 	struct usb_ep *(*match_ep)(struct usb_gadget *,
 			struct usb_endpoint_descriptor *,
 			struct usb_ss_ep_comp_descriptor *);
+	int	(*check_config)(struct usb_gadget *gadget, unsigned long ep_map);
 };
 
 /**
@@ -608,6 +609,7 @@  int usb_gadget_connect(struct usb_gadget *gadget);
 int usb_gadget_disconnect(struct usb_gadget *gadget);
 int usb_gadget_deactivate(struct usb_gadget *gadget);
 int usb_gadget_activate(struct usb_gadget *gadget);
+int usb_gadget_check_config(struct usb_gadget *gadget, unsigned long ep_map);
 #else
 static inline int usb_gadget_frame_number(struct usb_gadget *gadget)
 { return 0; }
@@ -631,6 +633,9 @@  static inline int usb_gadget_deactivate(struct usb_gadget *gadget)
 { return 0; }
 static inline int usb_gadget_activate(struct usb_gadget *gadget)
 { return 0; }
+static inline int usb_gadget_check_config(struct usb_gadget *gadget,
+					  unsigned long ep_map)
+{ return 0; }
 #endif /* CONFIG_USB_GADGET */
 
 /*-------------------------------------------------------------------------*/