| Message ID | 20241007104239.1624430-1-s-vadapalli@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Mattijs Korpershoek |
| Headers | show |
| Series | usb: gadget: cdns3: Fix cdns3_ep_config() by setting ep.maxpacket | expand |
On 07/10/2024 13:42, 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 by using usb_ep_set_maxpacket_limit(). > > 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. > > Patch has been tested on AM642-EVM. > > 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..b0b08768d9 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(). > + */ > + usb_ep_set_maxpacket_limit(&priv_ep->endpoint, max_packet_size); > + from include/linux/usb/gadget.h * struct usb_ep - device side representation of USB endpoint ... * @maxpacket:The maximum packet size used on this endpoint. The initial * value can sometimes be reduced (hardware allowing), according to * the endpoint descriptor used to configure the endpoint. * @maxpacket_limit:The maximum packet size value which can be handled by this * endpoint. It's set once by UDC driver when endpoint is initialized, and * should not be changed. Should not be confused with maxpacket. What you want to change is ep->maxpacket and not ep->maxpacket_limit as that is fixed for a particular UDC. how about? priv-ep->endpoint.maxpacket = max_packet_size; > ret = cdns3_ep_onchip_buffer_reserve(priv_dev, buffering + 1, > !!priv_ep->dir); > if (ret) {
On Mon, Oct 07, 2024 at 03:02:34PM +0300, Roger Quadros wrote: Hello Roger, > On 07/10/2024 13:42, 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). > > [...] > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > > index ac7e469469..b0b08768d9 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(). > > + */ > > + usb_ep_set_maxpacket_limit(&priv_ep->endpoint, max_packet_size); > > + > > from include/linux/usb/gadget.h > > * struct usb_ep - device side representation of USB endpoint > ... > * @maxpacket:The maximum packet size used on this endpoint. The initial > * value can sometimes be reduced (hardware allowing), according to > * the endpoint descriptor used to configure the endpoint. > * @maxpacket_limit:The maximum packet size value which can be handled by this > * endpoint. It's set once by UDC driver when endpoint is initialized, and > * should not be changed. Should not be confused with maxpacket. > > What you want to change is ep->maxpacket and not ep->maxpacket_limit as that is > fixed for a particular UDC. > > how about? > priv-ep->endpoint.maxpacket = max_packet_size; Yes, this is accurate. I will post the v2 patch with this change. Thank you for reviewing the patch. Regards, Siddharth.
On Mon, Oct 07, 2024 at 05:37:52PM +0530, Siddharth Vadapalli wrote: > On Mon, Oct 07, 2024 at 03:02:34PM +0300, Roger Quadros wrote: > > Hello Roger, [...] > > from include/linux/usb/gadget.h > > > > * struct usb_ep - device side representation of USB endpoint > > ... > > * @maxpacket:The maximum packet size used on this endpoint. The initial > > * value can sometimes be reduced (hardware allowing), according to > > * the endpoint descriptor used to configure the endpoint. > > * @maxpacket_limit:The maximum packet size value which can be handled by this > > * endpoint. It's set once by UDC driver when endpoint is initialized, and > > * should not be changed. Should not be confused with maxpacket. > > > > What you want to change is ep->maxpacket and not ep->maxpacket_limit as that is > > fixed for a particular UDC. > > > > how about? > > priv-ep->endpoint.maxpacket = max_packet_size; > > Yes, this is accurate. I will post the v2 patch with this change. Thank > you for reviewing the patch. I have implemented your feedback and have posted the v2 patch at: https://patchwork.ozlabs.org/project/uboot/patch/20241007121927.1680039-1-s-vadapalli@ti.com Regards, Siddharth.
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index ac7e469469..b0b08768d9 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(). + */ + usb_ep_set_maxpacket_limit(&priv_ep->endpoint, max_packet_size); + ret = cdns3_ep_onchip_buffer_reserve(priv_dev, buffering + 1, !!priv_ep->dir); if (ret) {
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 by using usb_ep_set_maxpacket_limit(). 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. Patch has been tested on AM642-EVM. Regards, Siddharth. drivers/usb/cdns3/gadget.c | 8 ++++++++ 1 file changed, 8 insertions(+)