Messages in this thread Patch in this message | | | Subject | Re: [RFC PATCH] usb: gadget: f_tcm: Added DMA32 flag while allocation of command buffer | From | Roger Quadros <> | Date | Thu, 14 Nov 2019 11:48:01 +0200 |
| |
Jayshri,
On 14/11/2019 04:50, Peter Chen wrote: > On 19-11-13 10:24:32, Jayshri Pawar wrote: >> There is a problem when function driver allocate memory for buffer >> used by DMA from outside dma_mask space. >> It appears during testing f_tcm driver with cdns3 controller. >> In the result cdns3 driver was not able to map virtual buffer to DMA. >> This fix should be improved depending on dma_mask associated with device. >> Adding GFP_DMA32 flag while allocationg command data buffer only for 32 >> bit controllers. > > Hi Jayshri, > > This issue should be fixed by setting DMA_MASK correctly for controller, > you can't limit user's memory region. At usb_ep_queue, the UDC driver > will call DMA MAP API, for Cadence, it is usb_gadget_map_request_by_dev. > For the system without SMMU (IO-MMU), it will use swiotlb to make sure > the data buffer used for DMA transfer is within DMA mask for controller, > There is a reserved low memory region for debounce buffer in swiotlb > use case. >
/** * struct usb_request - describes one i/o request * @buf: Buffer used for data. Always provide this; some controllers * only use PIO, or don't use DMA for some endpoints. * @dma: DMA address corresponding to 'buf'. If you don't set this * field, and the usb controller needs one, it is responsible * for mapping and unmapping the buffer. <snip> */
So if dma is not set in the usb_request then controller driver is responsible to do a dma_map of the buffer pointed by 'buf' before it attemps to do DMA. This should take care of DMA mask and swiotlb.
This patch is not correct.
cheers, -roger
> Peter > >> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> >> Signed-off-by: Jayshri Pawar <jpawar@cadence.com> >> --- >> drivers/usb/gadget/function/f_tcm.c | 20 ++++++++++++++------ >> include/linux/usb/gadget.h | 2 ++ >> 2 files changed, 16 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c >> index 36504931b2d1..a78d5fad3d84 100644 >> --- a/drivers/usb/gadget/function/f_tcm.c >> +++ b/drivers/usb/gadget/function/f_tcm.c >> @@ -213,7 +213,8 @@ static int bot_send_read_response(struct usbg_cmd *cmd) >> } >> >> if (!gadget->sg_supported) { >> - cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC); >> + cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC | >> + gadget->dma_flag); >> if (!cmd->data_buf) >> return -ENOMEM; >> >> @@ -257,7 +258,8 @@ static int bot_send_write_request(struct usbg_cmd *cmd) >> } >> >> if (!gadget->sg_supported) { >> - cmd->data_buf = kmalloc(se_cmd->data_length, GFP_KERNEL); >> + cmd->data_buf = kmalloc(se_cmd->data_length, GFP_KERNEL | >> + gadget->dma_flag); >> if (!cmd->data_buf) >> return -ENOMEM; >> >> @@ -305,6 +307,7 @@ static void bot_cmd_complete(struct usb_ep *ep, struct usb_request *req) >> static int bot_prepare_reqs(struct f_uas *fu) >> { >> int ret; >> + struct usb_gadget *gadget = fuas_to_gadget(fu); >> >> fu->bot_req_in = usb_ep_alloc_request(fu->ep_in, GFP_KERNEL); >> if (!fu->bot_req_in) >> @@ -327,7 +330,8 @@ static int bot_prepare_reqs(struct f_uas *fu) >> fu->bot_status.req->complete = bot_status_complete; >> fu->bot_status.csw.Signature = cpu_to_le32(US_BULK_CS_SIGN); >> >> - fu->cmd.buf = kmalloc(fu->ep_out->maxpacket, GFP_KERNEL); >> + fu->cmd.buf = kmalloc(fu->ep_out->maxpacket, GFP_KERNEL | >> + gadget->dma_flag); >> if (!fu->cmd.buf) >> goto err_buf; >> >> @@ -515,7 +519,8 @@ static int uasp_prepare_r_request(struct usbg_cmd *cmd) >> struct uas_stream *stream = cmd->stream; >> >> if (!gadget->sg_supported) { >> - cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC); >> + cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC | >> + gadget->dma_flag); >> if (!cmd->data_buf) >> return -ENOMEM; >> >> @@ -763,11 +768,13 @@ static int uasp_alloc_stream_res(struct f_uas *fu, struct uas_stream *stream) >> >> static int uasp_alloc_cmd(struct f_uas *fu) >> { >> + struct usb_gadget *gadget = fuas_to_gadget(fu); >> fu->cmd.req = usb_ep_alloc_request(fu->ep_cmd, GFP_KERNEL); >> if (!fu->cmd.req) >> goto err; >> >> - fu->cmd.buf = kmalloc(fu->ep_cmd->maxpacket, GFP_KERNEL); >> + fu->cmd.buf = kmalloc(fu->ep_cmd->maxpacket, GFP_KERNEL | >> + gadget->dma_flag); >> if (!fu->cmd.buf) >> goto err_buf; >> >> @@ -980,7 +987,8 @@ static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req) >> struct usb_gadget *gadget = fuas_to_gadget(fu); >> >> if (!gadget->sg_supported) { >> - cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC); >> + cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC | >> + gadget->dma_flag); >> if (!cmd->data_buf) >> return -ENOMEM; >> >> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h >> index 124462d65eac..d6c9cd222600 100644 >> --- a/include/linux/usb/gadget.h >> +++ b/include/linux/usb/gadget.h >> @@ -373,6 +373,7 @@ struct usb_gadget_ops { >> * @connected: True if gadget is connected. >> * @lpm_capable: If the gadget max_speed is FULL or HIGH, this flag >> * indicates that it supports LPM as per the LPM ECN & errata. >> + * @dma_flag: dma zone to be used for buffer allocation. >> * >> * Gadgets have a mostly-portable "gadget driver" implementing device >> * functions, handling all usb configurations and interfaces. Gadget >> @@ -427,6 +428,7 @@ struct usb_gadget { >> unsigned deactivated:1; >> unsigned connected:1; >> unsigned lpm_capable:1; >> + unsigned int dma_flag; >> }; >> #define work_to_gadget(w) (container_of((w), struct usb_gadget, work)) >> >> -- >> 2.20.1 >> >
-- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
| |