lkml.org 
[lkml]   [2019]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [RFC PATCH] usb: gadget: f_tcm: Added DMA32 flag while allocation of command buffer
From
Date
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

\
 
 \ /
  Last update: 2019-11-14 10:48    [W:0.099 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site