On 27/06/2023 20:26, Frank Li wrote: > > >> -----Original Message----- >> From: Roger Quadros <rogerq@xxxxxxxxxx> >> Sent: Tuesday, June 27, 2023 5:41 AM >> To: Ravi Gunasekaran <r-gunasekaran@xxxxxx>; Frank Li <frank.li@xxxxxxx> >> Cc: linux-usb@xxxxxxxxxxxxxxx; peter.chen@xxxxxxxxxx; >> pawell@xxxxxxxxxxx >> Subject: [EXT] Re: usb: cdns3: Onchip memory reservation for built-in >> gadgets >> >> Caution: This is an external email. Please take care when clicking links or >> opening attachments. When in doubt, report the message using the 'Report >> this email' button >> >> >> Hi, >> >> On 27/06/2023 15:56, Ravi Gunasekaran wrote: >>> Hi, >>> >>> Firstly, I'm not sure if it is alright to post queries this way. >>> If it is wrong, I apologize for it. Please let me know the right path/forum to >> ask the questions. >>> >>> This is regarding the commit >>> dce49449e04f usb: cdns3: allocate TX FIFO size according to composite EP >> number >>> >>> This commit introduced cdns3_gadget_check_config() which is invoked >> while binding gadget created via configfs and >>> also a logic to calculate ep_buf_size (which was CDNS3_EP_BUF_SIZE = 4). >>> >>> But for gadgets such as g_ether, g_cdc, the checks are not performed. And >> also for these legacy gadget drivers, >>> memory needs to be reserved for multiple IN end points and shared >> memory for OUT end points. So when ep_buf_size = 15, >>> the memory reservation fails, as it exceeds total onchip memory. >>> >>> So I was wondering if additional checks need to done in the cadence gadget >> driver or am I doing something wrong while >>> loading gadgets such as g_ether. >>> >> >> I think Ravi's concern is that. >> >> prior to commit dce49449e04f, priv_dev->ep_buf_size was fixed at 4. >> After commit dce49449e04f, it is at 0 if check_config() is not called e.g. >> in the legacy gadget cases (e.g. g_ether). >> So cdns3_ep_config() fails with dev_err(priv_dev->dev, "onchip mem is full, >> ep is invalid\n"). >> >> A potential fix might be to start with priv_dev->ep_buf_size = 4 >> instead of 0 so it works for legacy gadgets as well where check_config() is not >> invoked. > > I think it should fix at legacy driver by call usb_gadget_check_config. > Assume there are a UDC controller, which only 2 endpoint. g_cdc should > get failure. > commit dce49449e04f has broken legacy gadget drivers for CDNS3 UDC and that should be fixed first. No other UDC drivers implement .check_config() yet. UDC driver should start with a default sane configuration and work even if usb_gadget_check_config() is not called by gadget driver. -- cheers, -roger