Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752976AbcD0FnN (ORCPT ); Wed, 27 Apr 2016 01:43:13 -0400 Received: from mga02.intel.com ([134.134.136.20]:34823 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752641AbcD0FnM (ORCPT ); Wed, 27 Apr 2016 01:43:12 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,540,1455004800"; d="asc'?scan'208";a="692941445" From: Felipe Balbi To: Grygorii Strashko , Greg Kroah-Hartman Cc: linux-arm-kernel@lists.infradead.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Sekhar Nori , David Fisher , Catalin Marinas , "Thang Q. Nguyen" , Yoshihiro Shimoda Subject: Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev In-Reply-To: <571F2357.9080509@ti.com> References: <1461612094-30939-1-git-send-email-grygorii.strashko@ti.com> <87d1pcrj0t.fsf@intel.com> <571F2357.9080509@ti.com> User-Agent: Notmuch/0.21+96~g9bbc54b (http://notmuchmail.org) Emacs/25.0.90.3 (x86_64-pc-linux-gnu) Date: Wed, 27 Apr 2016 08:41:06 +0300 Message-ID: <878tzzpq19.fsf@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9184 Lines: 255 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Grygorii Strashko writes: > On 04/26/2016 09:17 AM, Felipe Balbi wrote: >>=20 >> Hi, >>=20 >> Grygorii Strashko writes: >>> Now not all DMA paremters configured properly for "xhci-hcd" platform >>> device which is created manually. For example: dma_pfn_offset, dam_ops >>> and iommu configuration will not corresponds "dwc3" devices >>> configuration. As result, this will cause problems like wrong DMA >>> addresses translation on platforms with LPAE enabled like Keystone 2. >>> >>> When platform is using DT boot mode the DMA configuration will be >>> parsed and applied from DT, so, to fix this issue, reuse >>> of_dma_configure() API and retrieve DMA configuartion for "xhci-hcd" >>> from DWC3 device node. >>=20 >> patch is incomplete. You left out non-DT users which might suffer from >> the same problem. >>=20 > > Honestly, I don't know how to fix it gracefully for non-DT case. > I can update commit message to mention that this is fix for DT case only. no, that won't do :-) There are other users for this driver and they are all "out-of-compliance" when it comes to DMA usage. Apparently, the desired behavior is to pass correct device to DMA API which the gadget side is already doing (see below). For the host side, the fix has to be more involved. Frankly, I'd prefer that DMA setup could be inherited from parent device, then it wouldn't really matter and a bunch of this could be simplified. Some sort of dma_inherit(struct device *dev, struct device *parent) would go a long way, IMHO. 8<-------------------------------- cut here ------------------------ commit 2725d6f974c4c268ae5fb746f8e3b33b76135aa8 Author: Felipe Balbi Date: Tue Apr 19 16:18:12 2016 +0300 usb: dwc3: use parent device for DMA =20=20=20=20 our parent device is the one which was initialized by either PCI or DeviceTree and that's the one which is configured properly for DMA access. =20=20=20=20 Instead of copying DMA bits from parent to child, let's just rely on parent device for the entire DMA API. =20=20=20=20 Signed-off-by: Felipe Balbi diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index c050a88c16d4..09e4ff71a50f 100644 =2D-- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -180,7 +180,7 @@ static void dwc3_frame_length_adjustment(struct dwc3 *d= wc, u32 fladj) static void dwc3_free_one_event_buffer(struct dwc3 *dwc, struct dwc3_event_buffer *evt) { =2D dma_free_coherent(dwc->dev, evt->length, evt->buf, evt->dma); + dma_free_coherent(dwc->dev->parent, evt->length, evt->buf, evt->dma); } =20 /** @@ -202,7 +202,7 @@ static struct dwc3_event_buffer *dwc3_alloc_one_event_b= uffer(struct dwc3 *dwc, =20 evt->dwc =3D dwc; evt->length =3D length; =2D evt->buf =3D dma_alloc_coherent(dwc->dev, length, + evt->buf =3D dma_alloc_coherent(dwc->dev->parent, length, &evt->dma, GFP_KERNEL); if (!evt->buf) return ERR_PTR(-ENOMEM); diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index 143deb420481..c78238dc76fc 100644 =2D-- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -967,7 +967,7 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc, u32 transfer_size =3D 0; u32 maxpacket; =20 =2D ret =3D usb_gadget_map_request(&dwc->gadget, &req->request, + ret =3D usb_gadget_map_request_by_dev(dwc->dev->parent, &req->request, dep->number); if (ret) { dwc3_trace(trace_dwc3_ep0, "failed to map request\n"); @@ -995,7 +995,7 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc, dwc->ep0_bounce_addr, transfer_size, DWC3_TRBCTL_CONTROL_DATA, false); } else { =2D ret =3D usb_gadget_map_request(&dwc->gadget, &req->request, + ret =3D usb_gadget_map_request_by_dev(dwc->dev->parent, &req->request, dep->number); if (ret) { dwc3_trace(trace_dwc3_ep0, "failed to map request\n"); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 88fd30bf0c46..6929775bde57 100644 =2D-- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -191,7 +191,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct d= wc3_request *req, if (dwc->ep0_bounced && dep->number =3D=3D 0) dwc->ep0_bounced =3D false; else =2D usb_gadget_unmap_request(&dwc->gadget, &req->request, + usb_gadget_unmap_request_by_dev(dwc->dev->parent, &req->request, req->direction); =20 trace_dwc3_gadget_giveback(req); @@ -335,7 +335,7 @@ static int dwc3_alloc_trb_pool(struct dwc3_ep *dep) if (dep->trb_pool) return 0; =20 =2D dep->trb_pool =3D dma_alloc_coherent(dwc->dev, + dep->trb_pool =3D dma_alloc_coherent(dwc->dev->parent, sizeof(struct dwc3_trb) * DWC3_TRB_NUM, &dep->trb_pool_dma, GFP_KERNEL); if (!dep->trb_pool) { @@ -351,7 +351,8 @@ static void dwc3_free_trb_pool(struct dwc3_ep *dep) { struct dwc3 *dwc =3D dep->dwc; =20 =2D dma_free_coherent(dwc->dev, sizeof(struct dwc3_trb) * DWC3_TRB_NUM, + dma_free_coherent(dwc->dev->parent, + sizeof(struct dwc3_trb) * DWC3_TRB_NUM, dep->trb_pool, dep->trb_pool_dma); =20 dep->trb_pool =3D NULL; @@ -972,7 +973,7 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *= dep, u16 cmd_param, * here and stop, unmap, free and del each of the linked * requests instead of what we do now. */ =2D usb_gadget_unmap_request(&dwc->gadget, &req->request, + usb_gadget_unmap_request_by_dev(dwc->dev->parent, &req->request, req->direction); list_del(&req->list); return ret; @@ -1057,7 +1058,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep= , struct dwc3_request *req) * This will also avoid Host cancelling URBs due to too * many NAKs. */ =2D ret =3D usb_gadget_map_request(&dwc->gadget, &req->request, + ret =3D usb_gadget_map_request_by_dev(dwc->dev->parent, &req->request, dep->direction); if (ret) return ret; @@ -2734,7 +2735,8 @@ int dwc3_gadget_init(struct dwc3 *dwc) { int ret; =20 =2D dwc->ctrl_req =3D dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req), + dwc->ctrl_req =3D dma_alloc_coherent(dwc->dev->parent, + sizeof(*dwc->ctrl_req), &dwc->ctrl_req_addr, GFP_KERNEL); if (!dwc->ctrl_req) { dev_err(dwc->dev, "failed to allocate ctrl request\n"); @@ -2742,7 +2744,8 @@ int dwc3_gadget_init(struct dwc3 *dwc) goto err0; } =20 =2D dwc->ep0_trb =3D dma_alloc_coherent(dwc->dev, sizeof(*dwc->ep0_trb) * 2, + dwc->ep0_trb =3D dma_alloc_coherent(dwc->dev->parent, + sizeof(*dwc->ep0_trb) * 2, &dwc->ep0_trb_addr, GFP_KERNEL); if (!dwc->ep0_trb) { dev_err(dwc->dev, "failed to allocate ep0 trb\n"); @@ -2756,7 +2759,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) goto err2; } =20 =2D dwc->ep0_bounce =3D dma_alloc_coherent(dwc->dev, + dwc->ep0_bounce =3D dma_alloc_coherent(dwc->dev->parent, DWC3_EP0_BOUNCE_SIZE, &dwc->ep0_bounce_addr, GFP_KERNEL); if (!dwc->ep0_bounce) { @@ -2828,18 +2831,18 @@ err5: =20 err4: dwc3_gadget_free_endpoints(dwc); =2D dma_free_coherent(dwc->dev, DWC3_EP0_BOUNCE_SIZE, + dma_free_coherent(dwc->dev->parent, DWC3_EP0_BOUNCE_SIZE, dwc->ep0_bounce, dwc->ep0_bounce_addr); =20 err3: kfree(dwc->setup_buf); =20 err2: =2D dma_free_coherent(dwc->dev, sizeof(*dwc->ep0_trb), + dma_free_coherent(dwc->dev->parent, sizeof(*dwc->ep0_trb), dwc->ep0_trb, dwc->ep0_trb_addr); =20 err1: =2D dma_free_coherent(dwc->dev, sizeof(*dwc->ctrl_req), + dma_free_coherent(dwc->dev->parent, sizeof(*dwc->ctrl_req), dwc->ctrl_req, dwc->ctrl_req_addr); =20 err0: @@ -2854,16 +2857,16 @@ void dwc3_gadget_exit(struct dwc3 *dwc) =20 dwc3_gadget_free_endpoints(dwc); =20 =2D dma_free_coherent(dwc->dev, DWC3_EP0_BOUNCE_SIZE, + dma_free_coherent(dwc->dev->parent, DWC3_EP0_BOUNCE_SIZE, dwc->ep0_bounce, dwc->ep0_bounce_addr); =20 kfree(dwc->setup_buf); kfree(dwc->zlp_buf); =20 =2D dma_free_coherent(dwc->dev, sizeof(*dwc->ep0_trb), + dma_free_coherent(dwc->dev->parent, sizeof(*dwc->ep0_trb), dwc->ep0_trb, dwc->ep0_trb_addr); =20 =2D dma_free_coherent(dwc->dev, sizeof(*dwc->ctrl_req), + dma_free_coherent(dwc->dev->parent, sizeof(*dwc->ctrl_req), dwc->ctrl_req, dwc->ctrl_req_addr); } =20 =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIbBAEBAgAGBQJXIFDyAAoJEIaOsuA1yqRELRYP93Zl6hLcNND209Hhm/hkECPQ Cj1EYx4w8bJAkYiN3xGcMudDqP3LzCWs5wrcGxvNk6+BvZbhnd1Et33B7OlRmsMH n0N9jk3OuOm9dXrWK6Ypqp5cSz5iWqXcauzVQR2EYrIv15lYhFlEMVIVSgUUXNUL IVGFxwDN0zD/KtdqZC6ioTgNdTtx4x2coQb9NB9wZccoUkrHjhMkrrNWFzTbSKNX DUHmiN+Pozpp9wpoDAHlTXmNo2tIezQiuqTTEu4IDbPolE4KAgY8/VWZwg+gxSTn LtvYie7RUBUZMcKu246lzRhOB1skU1/ln27Ha5pec8mQ7SzoPpVCMQ0qM6F36p66 +9WYrMcCRyzo0QEPCAOx6Unuv79YgLtxOOq8EhVJbn6EWPEopp6C4siDYCacpUx8 uss2hLp105kq2UIPho2qyD113X6It3bPHhxqSQZxSe320bZNJH3QgqkW8bgLJNqS Mfvn5I7KYpmQ64l/8UJapYFhpM6wV1KwRxyJ7Ta5s3hKrvzqllqEnAhJCl0nGhlP 4pkZaZ/+qkcvt4FwAb1Pt7XOiedq4drzdzAdj/+lJ2m1ynpe6T2cCE/YqD3opCqX uPIFQA4sKj8F8urJbjyxKGcL18uD014WxDJYuRWN7GvoONeIIkYeyS/cEa6OhXLk 3FO0WflC/zOysVg6wno= =Uj86 -----END PGP SIGNATURE----- --=-=-=--