* [PATCH 1/1] usb: xhci: setup packets don't need DMA mapping @ 2021-01-14 2:59 Daewoong Kim 2021-01-14 5:04 ` Peter Chen 0 siblings, 1 reply; 5+ messages in thread From: Daewoong Kim @ 2021-01-14 2:59 UTC (permalink / raw) To: linux-kernel, linux-usb Cc: allen.lkml, gustavoars, a.darwish, romain.perier, dvyukov, andreyknvl, mathias.nyman, gregkh, Daewoong Kim DMA mapping of urb->setup_packet is not necessary for xHCI host controllers. The xHCI specification says that Setup Stage TRB includes whole Setup Data; therefore, urb->setup_dma will not be used in the xhci HCD code. Signed-off-by: Daewoong Kim <daewoong00.kim@lge.com> --- drivers/usb/core/hcd.c | 4 +++- drivers/usb/host/xhci.c | 1 + include/linux/usb.h | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index ad5a0f405a75..b1f9eac93f0d 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1411,7 +1411,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, if (usb_endpoint_xfer_control(&urb->ep->desc)) { if (hcd->self.uses_pio_for_control) return ret; - if (hcd->localmem_pool) { + if (hcd->self.uses_pio_for_setup_pkt) { + ; /* do nothing */ + } else if (hcd->localmem_pool) { ret = hcd_alloc_coherent( urb->dev->bus, mem_flags, &urb->setup_dma, diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index e86940571b4c..c263aee82dc0 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -643,6 +643,7 @@ int xhci_run(struct usb_hcd *hcd) */ hcd->uses_new_polling = 1; + hcd->self.uses_pio_for_setup_pkt = 1; if (!usb_hcd_is_primary_hcd(hcd)) return xhci_run_finished(xhci); diff --git a/include/linux/usb.h b/include/linux/usb.h index 7d72c4e0713c..76600e8de414 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -430,6 +430,10 @@ struct usb_bus { * Does the host controller use PIO * for control transfers? */ + u8 uses_pio_for_setup_pkt; /* + * Does the host controller use PIO + * for setup packets? + */ u8 otg_port; /* 0, or number of OTG/HNP port */ unsigned is_b_host:1; /* true during some HNP roleswitches */ unsigned b_hnp_enable:1; /* OTG: did A-Host enable HNP? */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] usb: xhci: setup packets don't need DMA mapping 2021-01-14 2:59 [PATCH 1/1] usb: xhci: setup packets don't need DMA mapping Daewoong Kim @ 2021-01-14 5:04 ` Peter Chen 2021-01-14 18:00 ` Alan Stern 0 siblings, 1 reply; 5+ messages in thread From: Peter Chen @ 2021-01-14 5:04 UTC (permalink / raw) To: Daewoong Kim Cc: linux-kernel, linux-usb, allen.lkml, gustavoars, a.darwish, romain.perier, dvyukov, andreyknvl, mathias.nyman, gregkh On 21-01-14 11:59:07, Daewoong Kim wrote: > DMA mapping of urb->setup_packet is not necessary for xHCI host > controllers. The xHCI specification says that Setup Stage TRB includes > whole Setup Data; therefore, urb->setup_dma will not be used in the xhci > HCD code. > How about bypass map/unmap operation for xHCI control transfer directly? diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 91ab81c3fc79..0a0ab14b7638 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1374,7 +1374,8 @@ static int xhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, xhci = hcd_to_xhci(hcd); - if (xhci_urb_suitable_for_idt(urb)) + if (xhci_urb_suitable_for_idt(urb) || + (usb_endpoint_xfer_control(&urb->ep->desc))) return 0; if (xhci->quirks & XHCI_SG_TRB_CACHE_SIZE_QUIRK) { @@ -1389,6 +1390,9 @@ static void xhci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb) struct xhci_hcd *xhci; bool unmap_temp_buf = false; + if (usb_endpoint_xfer_control(&urb->ep->desc)) + return; + xhci = hcd_to_xhci(hcd); if (urb->num_sgs && (urb->transfer_flags & URB_DMA_MAP_SINGLE)) > Signed-off-by: Daewoong Kim <daewoong00.kim@lge.com> > --- > drivers/usb/core/hcd.c | 4 +++- > drivers/usb/host/xhci.c | 1 + > include/linux/usb.h | 4 ++++ > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index ad5a0f405a75..b1f9eac93f0d 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -1411,7 +1411,9 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, > if (usb_endpoint_xfer_control(&urb->ep->desc)) { > if (hcd->self.uses_pio_for_control) > return ret; > - if (hcd->localmem_pool) { > + if (hcd->self.uses_pio_for_setup_pkt) { > + ; /* do nothing */ > + } else if (hcd->localmem_pool) { > ret = hcd_alloc_coherent( > urb->dev->bus, mem_flags, > &urb->setup_dma, > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index e86940571b4c..c263aee82dc0 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -643,6 +643,7 @@ int xhci_run(struct usb_hcd *hcd) > */ > > hcd->uses_new_polling = 1; > + hcd->self.uses_pio_for_setup_pkt = 1; > if (!usb_hcd_is_primary_hcd(hcd)) > return xhci_run_finished(xhci); > > diff --git a/include/linux/usb.h b/include/linux/usb.h > index 7d72c4e0713c..76600e8de414 100644 > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -430,6 +430,10 @@ struct usb_bus { > * Does the host controller use PIO > * for control transfers? > */ > + u8 uses_pio_for_setup_pkt; /* > + * Does the host controller use PIO > + * for setup packets? > + */ > u8 otg_port; /* 0, or number of OTG/HNP port */ > unsigned is_b_host:1; /* true during some HNP roleswitches */ > unsigned b_hnp_enable:1; /* OTG: did A-Host enable HNP? */ > -- > 2.17.1 > -- Thanks, Peter Chen ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] usb: xhci: setup packets don't need DMA mapping 2021-01-14 5:04 ` Peter Chen @ 2021-01-14 18:00 ` Alan Stern 2021-01-15 0:32 ` Peter Chen 0 siblings, 1 reply; 5+ messages in thread From: Alan Stern @ 2021-01-14 18:00 UTC (permalink / raw) To: Peter Chen Cc: Daewoong Kim, linux-kernel, linux-usb, allen.lkml, gustavoars, a.darwish, romain.perier, dvyukov, andreyknvl, mathias.nyman, gregkh On Thu, Jan 14, 2021 at 01:04:02PM +0800, Peter Chen wrote: > On 21-01-14 11:59:07, Daewoong Kim wrote: > > DMA mapping of urb->setup_packet is not necessary for xHCI host > > controllers. The xHCI specification says that Setup Stage TRB includes > > whole Setup Data; therefore, urb->setup_dma will not be used in the xhci > > HCD code. > > > > How about bypass map/unmap operation for xHCI control transfer directly? > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 91ab81c3fc79..0a0ab14b7638 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1374,7 +1374,8 @@ static int xhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, > > xhci = hcd_to_xhci(hcd); > > - if (xhci_urb_suitable_for_idt(urb)) > + if (xhci_urb_suitable_for_idt(urb) || > + (usb_endpoint_xfer_control(&urb->ep->desc))) > return 0; Would this affect the map/unmap operations for the DATA packets in a control transfer, along with the SETUP packet? Alan Stern ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] usb: xhci: setup packets don't need DMA mapping 2021-01-14 18:00 ` Alan Stern @ 2021-01-15 0:32 ` Peter Chen 2021-01-18 1:07 ` Daewoong Kim 0 siblings, 1 reply; 5+ messages in thread From: Peter Chen @ 2021-01-15 0:32 UTC (permalink / raw) To: Alan Stern Cc: Daewoong Kim, linux-kernel, linux-usb, allen.lkml, gustavoars, a.darwish, romain.perier, dvyukov, andreyknvl, mathias.nyman, gregkh On 21-01-14 13:00:21, Alan Stern wrote: > On Thu, Jan 14, 2021 at 01:04:02PM +0800, Peter Chen wrote: > > On 21-01-14 11:59:07, Daewoong Kim wrote: > > > DMA mapping of urb->setup_packet is not necessary for xHCI host > > > controllers. The xHCI specification says that Setup Stage TRB includes > > > whole Setup Data; therefore, urb->setup_dma will not be used in the xhci > > > HCD code. > > > > > > > How about bypass map/unmap operation for xHCI control transfer directly? > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index 91ab81c3fc79..0a0ab14b7638 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -1374,7 +1374,8 @@ static int xhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, > > > > xhci = hcd_to_xhci(hcd); > > > > - if (xhci_urb_suitable_for_idt(urb)) > > + if (xhci_urb_suitable_for_idt(urb) || > > + (usb_endpoint_xfer_control(&urb->ep->desc))) > > return 0; > > Would this affect the map/unmap operations for the DATA packets in a > control transfer, along with the SETUP packet? > Oh, you are right, Alan. We do need map/unmap operation for DATA packet in control transfer. -- Thanks, Peter Chen ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] usb: xhci: setup packets don't need DMA mapping 2021-01-15 0:32 ` Peter Chen @ 2021-01-18 1:07 ` Daewoong Kim 0 siblings, 0 replies; 5+ messages in thread From: Daewoong Kim @ 2021-01-18 1:07 UTC (permalink / raw) To: hzpeterchen Cc: b-liu, allen.lkml, gustavoars, a.darwish, romain.perier, dvyukov, andreyknvl, mathias.nyman, gregkh, neidhard.kim, raxis.lim, linux-kernel, linux-usb Peter Chen, thanks for your comments. On Fri, 15 Jan 2021 08:32:51 +0800, Peter Chen wrote: > On 21-01-14 13:00:21, Alan Stern wrote: > > On Thu, Jan 14, 2021 at 01:04:02PM +0800, Peter Chen wrote: > > > On 21-01-14 11:59:07, Daewoong Kim wrote: > > > > DMA mapping of urb->setup_packet is not necessary for xHCI host > > > > controllers. The xHCI specification says that Setup Stage TRB includes > > > > whole Setup Data; therefore, urb->setup_dma will not be used in the xhci > > > > HCD code. > > > > > > > > > > How about bypass map/unmap operation for xHCI control transfer directly? > > > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > > index 91ab81c3fc79..0a0ab14b7638 100644 > > > --- a/drivers/usb/host/xhci.c > > > +++ b/drivers/usb/host/xhci.c > > > @@ -1374,7 +1374,8 @@ static int xhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, > > > > > > xhci = hcd_to_xhci(hcd); > > > > > > - if (xhci_urb_suitable_for_idt(urb)) > > > + if (xhci_urb_suitable_for_idt(urb) || > > > + (usb_endpoint_xfer_control(&urb->ep->desc))) > > > return 0; > > > > Would this affect the map/unmap operations for the DATA packets in a > > control transfer, along with the SETUP packet? > > > > Oh, you are right, Alan. We do need map/unmap operation for DATA > packet in control transfer. Is it possible to bypass map/unmap operations for MUSB devices' control transfer directly? If possible, the uses_pio_for_control field in struct usb_bus could be removed. diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index ad5a0f405a75..54061764ffda 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1409,8 +1409,6 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, */ if (usb_endpoint_xfer_control(&urb->ep->desc)) { - if (hcd->self.uses_pio_for_control) - return ret; if (hcd->localmem_pool) { ret = hcd_alloc_coherent( urb->dev->bus, mem_flags, diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index 30c5e7de0761..6eaff08cd1e4 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -2624,6 +2624,8 @@ static int musb_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, struct musb *musb = hcd_to_musb(hcd); int ret; + if (usb_endpoint_xfer_control(&urb->ep->desc)) + return 0; /* * The DMA engine in RTL1.8 and above cannot handle * DMA addresses that are not aligned to a 4 byte boundary. @@ -2700,7 +2702,6 @@ int musb_host_alloc(struct musb *musb) return -EINVAL; *musb->hcd->hcd_priv = (unsigned long) musb; - musb->hcd->self.uses_pio_for_control = 1; musb->hcd->uses_new_polling = 1; musb->hcd->has_tt = 1; diff --git a/include/linux/usb.h b/include/linux/usb.h index 7d72c4e0713c..f4b7d529f2eb 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -426,10 +426,6 @@ struct usb_bus { struct device *sysdev; /* as seen from firmware or bus */ int busnum; /* Bus number (in order of reg) */ const char *bus_name; /* stable id (PCI slot_name etc) */ - u8 uses_pio_for_control; /* - * Does the host controller use PIO - * for control transfers? - */ u8 otg_port; /* 0, or number of OTG/HNP port */ unsigned is_b_host:1; /* true during some HNP roleswitches */ unsigned b_hnp_enable:1; /* OTG: did A-Host enable HNP? */ -- With Best Regards, Daewoong Kim ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-18 1:08 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-01-14 2:59 [PATCH 1/1] usb: xhci: setup packets don't need DMA mapping Daewoong Kim 2021-01-14 5:04 ` Peter Chen 2021-01-14 18:00 ` Alan Stern 2021-01-15 0:32 ` Peter Chen 2021-01-18 1:07 ` Daewoong Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox