Re: [PATCH] usb: cdns3: cdns3-gadget: Fix cdns3_ep_config() by setting ep.maxpacket

[Roger Quadros <rogerq@kernel.org>]

Hi Siddharth,

On 11/10/2024 08:45, Siddharth Vadapalli wrote:
> The function cdns3_ep_config() calculates the maximum packet size based
> on the Endpoint Type and the Gadget Speed and stores it in the variable
> "max_packet_size". This value is then programmed in the USB Controller
> for the corresponding Endpoint. This may result in a mismatch between
> the maximum packet size programmed in the USB controller and the maximum
> packet size seen by the UDC Core via "maxpacket" member of "struct usb_ep".
> Additionally, since TD_SIZE as well as TOTAL_TDL are calculated in
> cdns3_ep_run_transfer() on the basis of the maximum packet size stored in
> the "maxpacket" member of "struct usb_ep", it may lead to incorrect values
> of TD_SIZE and TOTAL_TDL when compared with what the USB controller
> actually expects (max_packet_size). This also applies to the calculation of
> TDL in cdns3_ep_run_stream_transfer().

Could you please mention here what use case was broken due to this?

> 
> Fix this.
> 
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> 
> Hello,
> 
> This patch is based on commit
> 1d227fcc7222 Merge tag 'net-6.12-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
> of Mainline Linux.
> 
> Regards,
> Siddharth.
> 
>  drivers/usb/cdns3/cdns3-gadget.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
> index fd1beb10bba7..e89bd248e81d 100644
> --- a/drivers/usb/cdns3/cdns3-gadget.c
> +++ b/drivers/usb/cdns3/cdns3-gadget.c
> @@ -2145,6 +2145,16 @@ int cdns3_ep_config(struct cdns3_endpoint *priv_ep, bool enable)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * The Endpoint is configured to handle a maximum packet size of
> +	 * max_packet_size. Hence, set priv_ep->endpoint.maxpacket to
> +	 * max_packet_size. This is necessary to ensure that TD_SIZE and
> +	 * TOTAL_TDL are calculated correctly in cdns3_ep_run_transfer(),
> +	 * and also to ensure that TDL is calculated correctly in
> +	 * cdns3_ep_run_stream_transfer().
> +	 */
> +	priv_ep->endpoint.maxpacket = max_packet_size;
> +
>  	if (max_packet_size == 1024)
>  		priv_ep->trb_burst_size = 128;
>  	else if (max_packet_size >= 512)

-- 
cheers,
-roger