Re: [PATCH v2 2/3] usb: gadget: composite: Support eUSB2v2 bulk MPS update request

[Selvarasu Ganesan <selvarasu.g@samsung.com>]

On 9/22/2026 1:46 PM, Pawel Laszczak wrote:
>> On 8/26/2026 4:54 PM, Pawel Laszczak via B4 Relay wrote:
>>> From: Pawel Laszczak <pawell@cadence.com>
>>>
>>> Add support for eUSB2v2 1024-byte Bulk MPS negotiation to the Gadget
>>> Composite framework.
>>>
>>> If 'gadget->is_eusb2v2' is set, force bcdUSB to 0x0230 and bMaxPacketSize0
>>> to 64 bytes. Handle the USB_DEVICE_BULK_MAX_PACKET_UPDATE feature
>> request
>>> by introducing eusb2_update_mps_bulk(), which dynamically updates the
>>> wMaxPacketSize of all HS Bulk endpoint descriptors to 1024 bytes before
>>> the device is configured.
>>>
>>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>>> ---
>>> Changes in v2:
>>> - composite.c: CLEAR_FEATURE(BULK_MAX_PACKET_UPDATE) is not defined in
>> the
>>>     eUSB2v2 spec; stall it instead of incorrectly setting 1024-byte mode.
>>> - restore MPS in __composite_disconnect
>>> ---
>>>    drivers/usb/gadget/composite.c | 68
>> ++++++++++++++++++++++++++++++++++++++----
>>>    include/linux/usb/gadget.h     |  2 ++
>>>    2 files changed, 64 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>>> index dc3664374596..f0246e25f2b5 100644
>>> --- a/drivers/usb/gadget/composite.c
>>> +++ b/drivers/usb/gadget/composite.c
>>> @@ -924,6 +924,48 @@ static void device_qual(struct usb_composite_dev
>> *cdev)
>>>    }
>>>
>>>    /*-------------------------------------------------------------------------*/
>>> +static void eusb2_update_ep_mps(struct usb_descriptor_header *header,
>> __le16 mps)
>>> +{
>>> +	struct usb_endpoint_descriptor *epd;
>>> +
>>> +	if (header->bDescriptorType != USB_DT_ENDPOINT)
>>> +		return;
>>> +
>>> +	epd = (void *)header;
>>> +	if (usb_endpoint_xfer_bulk(epd))
>>> +		epd->wMaxPacketSize = mps;
>>> +}
>>> +
>>> +static int eusb2_update_mps_bulk(struct usb_composite_dev *cdev, bool
>> set)
>>> +{
>>> +	__le16 mps = cpu_to_le16(set ? 1024 : 512);
>>> +	struct usb_gadget *gadget = cdev->gadget;
>>> +	struct usb_configuration *config;
>>> +	struct usb_function *f;
>>> +
>>> +	if (!gadget->is_eusb2v2)
>>> +		return -EINVAL;
>>> +
>>> +	if (set && gadget->state >= USB_STATE_CONFIGURED)
>>> +		return -EINVAL;
>>> +
>>> +	list_for_each_entry(config, &cdev->configs, list) {
>>> +		if (!config->highspeed)
>>> +			continue;
>>> +
>>> +		list_for_each_entry(f, &config->functions, list) {
>>> +			struct usb_descriptor_header **desc = f-
>>> hs_descriptors;
>>> +
>>> +			if (!desc)
>>> +				continue;
>>> +
>>> +			for (; *desc; desc++)
>>> +				eusb2_update_ep_mps(*desc, mps);
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>>
>>>    static void reset_config(struct usb_composite_dev *cdev)
>>>    {
>>> @@ -971,8 +1013,10 @@ static int set_config(struct usb_composite_dev
>> *cdev,
>>>    		if (result < 0)
>>>    			goto done;
>>>    	} else { /* Zero configuration value - need to reset the config */
>>> -		if (cdev->config)
>>> +		if (cdev->config) {
>>>    			reset_config(cdev);
>>> +			eusb2_update_mps_bulk(cdev, false);
>>> +		}
>>>    		result = 0;
>>>    	}
>>>
>>> @@ -1807,7 +1851,11 @@ composite_setup(struct usb_gadget *gadget,
>> const struct usb_ctrlrequest *ctrl)
>>>    				count_configs(cdev, USB_DT_DEVICE);
>>>    			cdev->desc.bMaxPacketSize0 =
>>>    				cdev->gadget->ep0->maxpacket;
>>> -			if (gadget_is_superspeed(gadget)) {
>>> +
>>> +			if (gadget->is_eusb2v2) {
>>> +				cdev->desc.bcdUSB = cpu_to_le16(0x0230);
>>> +				cdev->desc.bMaxPacketSize0 = 64;
>>> +			} else if (gadget_is_superspeed(gadget)) {
>>>    				if (gadget->speed >= USB_SPEED_SUPER) {
>>>    					cdev->desc.bcdUSB =
>> cpu_to_le16(0x0320);
>>>    					cdev->desc.bMaxPacketSize0 = 9;
>>> @@ -2005,12 +2053,19 @@ composite_setup(struct usb_gadget *gadget,
>> const struct usb_ctrlrequest *ctrl)
>>>    	 */
>>>    	case USB_REQ_CLEAR_FEATURE:
>>>    	case USB_REQ_SET_FEATURE:
>>> -		if (!gadget_is_superspeed(gadget))
>>> -			goto unknown;
>>> -		if (ctrl->bRequestType != (USB_DIR_OUT |
>> USB_RECIP_INTERFACE))
>>> -			goto unknown;
>>>    		switch (w_value) {
>>> +		case USB_DEVICE_BULK_MAX_PACKET_UPDATE:
>>> +			if (ctrl->bRequestType != (USB_DIR_OUT |
>> USB_RECIP_DEVICE))
>>> +				goto unknown;
>>> +			if (ctrl->bRequest != USB_REQ_SET_FEATURE)
>>> +				goto unknown;
>>> +			value = eusb2_update_mps_bulk(cdev, true);
>> Here both USB_REQ_CLEAR_FEATURE and USB_REQ_SET_FEATURE fall through
>> to
>> the same switch, and USB_DEVICE_BULK_MAX_PACKET_UPDATE unconditionally
>> calls eusb2_update_mps_bulk(cdev, true) regardless of which request type
>> was received.
>>
>> The CLEAR_FEATURE may request should revert the MPS back to the default
>> by calling eusb2_update_mps_bulk(cdev, false), but instead it performs
>> the same action as SET_FEATURE.
>>
>> Is this required distinguished using ctrl->bRequest to check whether
>> this is a SET or CLEAR operation?
> Hi Selva,
>
> Thank you for the review.
>
> The eUSB2v2 specification does not define CLEAR_FEATURE for the
> BULK_MAX_PACKET_UPDATE feature selector, so the device is expected to
> respond with a STALL in that case.
>
> The check on line:
>
> if (ctrl->bRequest != USB_REQ_SET_FEATURE)
>          goto unknown;
>
> already handles this: when CLEAR_FEATURE falls through to the
> USB_DEVICE_BULK_MAX_PACKET_UPDATE case, it hits this condition and
> jumps to 'unknown', which results in a STALL being returned to the
> host. So eusb2_update_mps_bulk(cdev, true) is never called for
> CLEAR_FEATURE.


Hi Pawel,

Thanks for the explanation. Its looks good to me.

It would be nice to have a brief mention of the CLEAR_FEATURE behavior 
and MPS restoration in the commit message just for future reference, but 
I'll leave it up to you if you think it's necessary.

Thanks,
Selva
>
> MPS restoration to 512 bytes is handled separately on disconnect and
> deconfiguration events (__composite_disconnect and set_config).
>
> Thanks,
> Pawel
>>
>> Thanks,
>> Selva
>>> +			break;
>>>    		case USB_INTRF_FUNC_SUSPEND:
>>> +			if (!gadget_is_superspeed(gadget))
>>> +				goto unknown;
>>> +			if (ctrl->bRequestType != (USB_DIR_OUT |
>> USB_RECIP_INTERFACE))
>>> +				goto unknown;
>>>    			if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
>>>    				break;
>>>    			f = cdev->config->interface[intf];
>>> @@ -2293,6 +2348,7 @@ static void __composite_disconnect(struct
>> usb_gadget *gadget)
>>>    	 * disconnect callbacks?
>>>    	 */
>>>    	spin_lock_irqsave(&cdev->lock, flags);
>>> +	eusb2_update_mps_bulk(cdev, false);
>>>    	cdev->suspended = 0;
>>>    	if (cdev->config)
>>>    		reset_config(cdev);
>>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>> index 8285b19a25e0..3c554fe95f87 100644
>>> --- a/include/linux/usb/gadget.h
>>> +++ b/include/linux/usb/gadget.h
>>> @@ -420,6 +420,7 @@ struct usb_gadget_ops {
>>>     * @wakeup_armed: True if gadget is armed by the host for remote wakeup.
>>>     * @irq: the interrupt number for device controller.
>>>     * @id_number: a unique ID number for ensuring that gadget names are
>> distinct
>>> + * @is_eusb2v2: True if controller is Embedded usb2.
>>>     *
>>>     * Gadgets have a mostly-portable "gadget driver" implementing device
>>>     * functions, handling all usb configurations and interfaces.  Gadget
>>> @@ -483,6 +484,7 @@ struct usb_gadget {
>>>    	unsigned			lpm_capable:1;
>>>    	unsigned			wakeup_capable:1;
>>>    	unsigned			wakeup_armed:1;
>>> +	unsigned			is_eusb2v2:1;
>>>    	int				irq;
>>>    	int				id_number;
>>>    };
>>>