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

From: Roger Quadros
Date: Fri Oct 11 2024 - 06:39:23 EST


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@xxxxxx>
> ---
>
> 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