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

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 is 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 an incorrect value of TD_SIZE when compared with what the USB controller actually expects (max_packet_size).
Fix this.
Fixes: 7e91f6ccdc84 ("usb: Add Cadence USB3 host and gadget driver") Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com ---
Hello,
This patch is based on commit dd4d130c8e clk: renesas: rcar-gen3: Fix SSCG caching replacement with MDSEL/PE caching of the master branch of U-Boot.
v1: https://patchwork.ozlabs.org/project/uboot/patch/20241007104239.1624430-1-s-... Changes since v1: - Based on Roger's feedback at: https://patchwork.ozlabs.org/project/uboot/patch/20241007104239.1624430-1-s-... rather than using usb_ep_set_maxpacket_limit() which changes "maxpacket_limit" in addition to "maxpacket", only update the "maxpacket" member of usb_ep.
Regards, Siddharth.
drivers/usb/cdns3/gadget.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index ac7e469469..a30c40ef80 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -1637,6 +1637,14 @@ void cdns3_ep_config(struct cdns3_endpoint *priv_ep) else priv_ep->trb_burst_size = 16;
+ /* + * 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 the TD_SIZE + * is calculated correctly in cdns3_ep_run_transfer(). + */ + priv_ep->endpoint.maxpacket = max_packet_size; + ret = cdns3_ep_onchip_buffer_reserve(priv_dev, buffering + 1, !!priv_ep->dir); if (ret) {

On 07/10/2024 15:19, 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 is 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 an incorrect value of TD_SIZE when compared with what the USB controller actually expects (max_packet_size).
Fix this.
Fixes: 7e91f6ccdc84 ("usb: Add Cadence USB3 host and gadget driver") Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
Reviewed-by: Roger Quadros rogerq@kernel.org

Hi Siddharth,
Thank you for the patch.
On lun., oct. 07, 2024 at 17:49, Siddharth Vadapalli s-vadapalli@ti.com 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 is 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 an incorrect value of TD_SIZE when compared with what the USB controller actually expects (max_packet_size).
Fix this.
Fixes: 7e91f6ccdc84 ("usb: Add Cadence USB3 host and gadget driver") Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Hello,
This patch is based on commit dd4d130c8e clk: renesas: rcar-gen3: Fix SSCG caching replacement with MDSEL/PE caching of the master branch of U-Boot.
v1: https://patchwork.ozlabs.org/project/uboot/patch/20241007104239.1624430-1-s-... Changes since v1:
- Based on Roger's feedback at: https://patchwork.ozlabs.org/project/uboot/patch/20241007104239.1624430-1-s-... rather than using usb_ep_set_maxpacket_limit() which changes "maxpacket_limit" in addition to "maxpacket", only update the "maxpacket" member of usb_ep.
Regards, Siddharth.
drivers/usb/cdns3/gadget.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index ac7e469469..a30c40ef80 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -1637,6 +1637,14 @@ void cdns3_ep_config(struct cdns3_endpoint *priv_ep) else priv_ep->trb_burst_size = 16;
- /*
* 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 the TD_SIZE
* is calculated correctly in cdns3_ep_run_transfer().
*/
- priv_ep->endpoint.maxpacket = max_packet_size;
- ret = cdns3_ep_onchip_buffer_reserve(priv_dev, buffering + 1, !!priv_ep->dir); if (ret) {
-- 2.40.1

Hi,
On Mon, 07 Oct 2024 17:49:27 +0530, 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 is 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 an incorrect value of TD_SIZE when compared with what the USB controller actually expects (max_packet_size).
[...]
Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu)
[1/1] usb: gadget: cdns3: Fix cdns3_ep_config() by setting ep.maxpacket https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/c54c72dec794809...
-- Mattijs
participants (3)
-
Mattijs Korpershoek
-
Roger Quadros
-
Siddharth Vadapalli