* [PATCH 0/4] Some fixes for PCI, mvneta, xHCI @ 2015-04-10 1:01 Sebastian Hesselbarth 2015-04-10 1:01 ` [PATCH 1/4] pci: fix device registration for directly attached devices Sebastian Hesselbarth ` (3 more replies) 0 siblings, 4 replies; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-10 1:01 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: Thomas Petazzoni, barebox, Uwe Kleine-König Before adding support for the Armada XP based Lenovo ix4 NAS, I split off some fixes/cleanups that I stumbled upon. Patch 1 fixes PCI device registration for non-bridge devices. Patch 2 fixes an outstanding transmit issue with mvneta that was reported by Uwe. It fixes mvneta on ix4, so I guess it should also fix it for his RN104. Patch 3 removes some unecessary DMA ops that where introduced during DMA op conversion. Patch 4 adds DMA ops for the data buffers used in xHCI HCD code. Sebastian Hesselbarth (4): pci: fix device registration for directly attached devices net: mvneta: Fix transmit errors due to dirty txdesc net: mvneta: Remove unnecessary DMA ops USB: xHCI: Sync non-coherent DMA buffers drivers/net/mvneta.c | 6 +----- drivers/pci/pci.c | 1 + drivers/usb/host/xhci-hcd.c | 6 ++++++ 3 files changed, 8 insertions(+), 5 deletions(-) --- Cc: barebox@lists.infradead.org Cc: Lucas Stach <dev@lynxeye.de> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> -- 2.1.0 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/4] pci: fix device registration for directly attached devices 2015-04-10 1:01 [PATCH 0/4] Some fixes for PCI, mvneta, xHCI Sebastian Hesselbarth @ 2015-04-10 1:01 ` Sebastian Hesselbarth 2015-04-10 7:15 ` Lucas Stach 2015-04-10 1:01 ` [PATCH 2/4] net: mvneta: Fix transmit errors due to dirty txdesc Sebastian Hesselbarth ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-10 1:01 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox Commit b8a1bb1dd215770670108fe5b0de0e5e137bf8fd ("pci: defer device registration until after bridge setup") removed pci_register_device() from setup_device() to allow bridges to be registered before attached sub-devices. This breaks normal registration for devices that are not connected through a bridge. Fix this by calling pci_register_device() for normal devices again. Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> --- Cc: barebox@lists.infradead.org Cc: Lucas Stach <dev@lynxeye.de> --- drivers/pci/pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d6e95c3ec894..c633d138ca66 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -344,6 +344,7 @@ unsigned int pci_scan_bus(struct pci_bus *bus) dev->rom_address = (l == 0xffffffff) ? 0 : l; setup_device(dev, 6); + pci_register_device(dev); break; case PCI_HEADER_TYPE_BRIDGE: setup_device(dev, 2); -- 2.1.0 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] pci: fix device registration for directly attached devices 2015-04-10 1:01 ` [PATCH 1/4] pci: fix device registration for directly attached devices Sebastian Hesselbarth @ 2015-04-10 7:15 ` Lucas Stach 2015-04-10 8:24 ` Sebastian Hesselbarth 0 siblings, 1 reply; 18+ messages in thread From: Lucas Stach @ 2015-04-10 7:15 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox Am Freitag, den 10.04.2015, 03:01 +0200 schrieb Sebastian Hesselbarth: > Commit b8a1bb1dd215770670108fe5b0de0e5e137bf8fd > ("pci: defer device registration until after bridge setup") > removed pci_register_device() from setup_device() to allow > bridges to be registered before attached sub-devices. > > This breaks normal registration for devices that are not > connected through a bridge. Fix this by calling pci_register_device() > for normal devices again. > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> NACK: This again breaks devices that are connected through a bridge if the bridge configuration is invalid at startup. There is already a patch on the list which should fix this problem "pci: make sure to activate devices on the root bus". Could you please test this instead? > --- > Cc: barebox@lists.infradead.org > Cc: Lucas Stach <dev@lynxeye.de> > --- > drivers/pci/pci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index d6e95c3ec894..c633d138ca66 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -344,6 +344,7 @@ unsigned int pci_scan_bus(struct pci_bus *bus) > dev->rom_address = (l == 0xffffffff) ? 0 : l; > > setup_device(dev, 6); > + pci_register_device(dev); > break; > case PCI_HEADER_TYPE_BRIDGE: > setup_device(dev, 2); _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] pci: fix device registration for directly attached devices 2015-04-10 7:15 ` Lucas Stach @ 2015-04-10 8:24 ` Sebastian Hesselbarth 0 siblings, 0 replies; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-10 8:24 UTC (permalink / raw) To: Lucas Stach; +Cc: barebox On 10.04.2015 09:15, Lucas Stach wrote: > Am Freitag, den 10.04.2015, 03:01 +0200 schrieb Sebastian Hesselbarth: >> Commit b8a1bb1dd215770670108fe5b0de0e5e137bf8fd >> ("pci: defer device registration until after bridge setup") >> removed pci_register_device() from setup_device() to allow >> bridges to be registered before attached sub-devices. >> >> This breaks normal registration for devices that are not >> connected through a bridge. Fix this by calling pci_register_device() >> for normal devices again. > >> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> > > NACK: This again breaks devices that are connected through a bridge if > the bridge configuration is invalid at startup. > > There is already a patch on the list which should fix this problem "pci: > make sure to activate devices on the root bus". > Could you please test this instead? Thanks for the hint, the patch on the list indeed fixes root bus attached device registration. I've replied with a Tested-by. Please drop this patch. Sebastian >> --- >> Cc: barebox@lists.infradead.org >> Cc: Lucas Stach <dev@lynxeye.de> >> --- >> drivers/pci/pci.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index d6e95c3ec894..c633d138ca66 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -344,6 +344,7 @@ unsigned int pci_scan_bus(struct pci_bus *bus) >> dev->rom_address = (l == 0xffffffff) ? 0 : l; >> >> setup_device(dev, 6); >> + pci_register_device(dev); >> break; >> case PCI_HEADER_TYPE_BRIDGE: >> setup_device(dev, 2); > > _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/4] net: mvneta: Fix transmit errors due to dirty txdesc 2015-04-10 1:01 [PATCH 0/4] Some fixes for PCI, mvneta, xHCI Sebastian Hesselbarth 2015-04-10 1:01 ` [PATCH 1/4] pci: fix device registration for directly attached devices Sebastian Hesselbarth @ 2015-04-10 1:01 ` Sebastian Hesselbarth 2015-04-13 6:43 ` Sascha Hauer 2015-04-10 1:01 ` [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops Sebastian Hesselbarth 2015-04-10 1:01 ` [PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers Sebastian Hesselbarth 3 siblings, 1 reply; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-10 1:01 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: Thomas Petazzoni, barebox, Uwe Kleine-König Marvell Neta's transmit descriptor (txdesc) is allocated by dma_alloc_coherent() but not zeroed before calling mvneta_send the first time. This can cause spurious transmit errors due to improperly set bits in txdesc's cmd_sts field. Fix initial transmit errors by always writing whole cmd_sts field instead of ORing the bits. Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> --- Cc: barebox@lists.infradead.org Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/net/mvneta.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/mvneta.c b/drivers/net/mvneta.c index d4c8a2c68dd2..3be2ec531fb1 100644 --- a/drivers/net/mvneta.c +++ b/drivers/net/mvneta.c @@ -396,7 +396,7 @@ static int mvneta_send(struct eth_device *edev, void *data, int len) dma_sync_single_for_device((unsigned long)data, len, DMA_TO_DEVICE); /* Fill the Tx descriptor */ - txdesc->cmd_sts |= MVNETA_TX_L4_CSUM_NOT | MVNETA_TXD_FLZ_DESC; + txdesc->cmd_sts = MVNETA_TX_L4_CSUM_NOT | MVNETA_TXD_FLZ_DESC; txdesc->buf_ptr = (u32)data; txdesc->byte_cnt = len; -- 2.1.0 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] net: mvneta: Fix transmit errors due to dirty txdesc 2015-04-10 1:01 ` [PATCH 2/4] net: mvneta: Fix transmit errors due to dirty txdesc Sebastian Hesselbarth @ 2015-04-13 6:43 ` Sascha Hauer 0 siblings, 0 replies; 18+ messages in thread From: Sascha Hauer @ 2015-04-13 6:43 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: Thomas Petazzoni, barebox, Uwe Kleine-König On Fri, Apr 10, 2015 at 03:01:52AM +0200, Sebastian Hesselbarth wrote: > Marvell Neta's transmit descriptor (txdesc) is allocated by dma_alloc_coherent() > but not zeroed before calling mvneta_send the first time. This can cause spurious > transmit errors due to improperly set bits in txdesc's cmd_sts field. > > Fix initial transmit errors by always writing whole cmd_sts field instead of ORing > the bits. > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> > --- > Cc: barebox@lists.infradead.org > Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > drivers/net/mvneta.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied this one, thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops 2015-04-10 1:01 [PATCH 0/4] Some fixes for PCI, mvneta, xHCI Sebastian Hesselbarth 2015-04-10 1:01 ` [PATCH 1/4] pci: fix device registration for directly attached devices Sebastian Hesselbarth 2015-04-10 1:01 ` [PATCH 2/4] net: mvneta: Fix transmit errors due to dirty txdesc Sebastian Hesselbarth @ 2015-04-10 1:01 ` Sebastian Hesselbarth 2015-04-10 7:23 ` Lucas Stach 2015-04-10 1:01 ` [PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers Sebastian Hesselbarth 3 siblings, 1 reply; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-10 1:01 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: Thomas Petazzoni, barebox Commit a76c62f80d95860e6c5904ab5cb91667c43f61eb ("net: mvneta: convert to streaming DMA ops") converted explicit ARM cache flushes to streaming DMA calls. However, in mvneta_send() we are not interested in the sent data buffer anymore. Also, in mvneta_recv() the device does not care about received data buffer. Remove unnecessary dma_sync_single_for_cpu() in mvneta_send() and dma_sync_single_for_device() in mvneta_recv(). Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> --- Cc: barebox@lists.infradead.org Cc: Lucas Stach <dev@lynxeye.de> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/net/mvneta.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/mvneta.c b/drivers/net/mvneta.c index 3be2ec531fb1..e1c7f15210e4 100644 --- a/drivers/net/mvneta.c +++ b/drivers/net/mvneta.c @@ -409,7 +409,6 @@ static int mvneta_send(struct eth_device *edev, void *data, int len) * the Tx port status register (PTXS). */ ret = wait_on_timeout(TRANSFER_TIMEOUT, !mvneta_pending_tx(priv)); - dma_sync_single_for_cpu((unsigned long)data, len, DMA_TO_DEVICE); if (ret) { dev_err(&edev->dev, "transmit timeout\n"); return ret; @@ -468,9 +467,6 @@ static int mvneta_recv(struct eth_device *edev) rxdesc->data_size - MVNETA_MH_SIZE); ret = 0; - dma_sync_single_for_device((unsigned long)rxdesc->buf_phys_addr, - ALIGN(PKTSIZE, 8), DMA_FROM_DEVICE); - recv_err: /* reset this and get next rx descriptor*/ rxdesc->data_size = 0; -- 2.1.0 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops 2015-04-10 1:01 ` [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops Sebastian Hesselbarth @ 2015-04-10 7:23 ` Lucas Stach 2015-04-10 8:18 ` Sebastian Hesselbarth 0 siblings, 1 reply; 18+ messages in thread From: Lucas Stach @ 2015-04-10 7:23 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: Thomas Petazzoni, barebox Am Freitag, den 10.04.2015, 03:01 +0200 schrieb Sebastian Hesselbarth: > Commit a76c62f80d95860e6c5904ab5cb91667c43f61eb > ("net: mvneta: convert to streaming DMA ops") > converted explicit ARM cache flushes to streaming DMA calls. > > However, in mvneta_send() we are not interested in the sent data buffer > anymore. Also, in mvneta_recv() the device does not care about received > data buffer. > > Remove unnecessary dma_sync_single_for_cpu() in mvneta_send() and > dma_sync_single_for_device() in mvneta_recv(). > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> NACK: see below. > --- > Cc: barebox@lists.infradead.org > Cc: Lucas Stach <dev@lynxeye.de> > Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > drivers/net/mvneta.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/net/mvneta.c b/drivers/net/mvneta.c > index 3be2ec531fb1..e1c7f15210e4 100644 > --- a/drivers/net/mvneta.c > +++ b/drivers/net/mvneta.c > @@ -409,7 +409,6 @@ static int mvneta_send(struct eth_device *edev, void *data, int len) > * the Tx port status register (PTXS). > */ > ret = wait_on_timeout(TRANSFER_TIMEOUT, !mvneta_pending_tx(priv)); > - dma_sync_single_for_cpu((unsigned long)data, len, DMA_TO_DEVICE); This makes sure the CPU has a consistent view of memory. If something got speculatively loaded into the cache you will write out invalid data on the next send. > if (ret) { > dev_err(&edev->dev, "transmit timeout\n"); > return ret; > @@ -468,9 +467,6 @@ static int mvneta_recv(struct eth_device *edev) > rxdesc->data_size - MVNETA_MH_SIZE); > ret = 0; > > - dma_sync_single_for_device((unsigned long)rxdesc->buf_phys_addr, > - ALIGN(PKTSIZE, 8), DMA_FROM_DEVICE); > - The buffer is reused for the next receive operation. This isn't syncing the data to the device, but is actually a cache invalidate to make sure there is no pending cache writeback that may corrupt your received data. > recv_err: > /* reset this and get next rx descriptor*/ > rxdesc->data_size = 0; The DMA API has a notion of buffer ownership. Accessing a buffer without transferring the ownership to the appropriate entity (cpu or device) is illegal. Using the DMA API in a non balanced manner is skipping the ownership transfer. Regards, Lucas _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops 2015-04-10 7:23 ` Lucas Stach @ 2015-04-10 8:18 ` Sebastian Hesselbarth 2015-04-10 16:09 ` Jan Lübbe 0 siblings, 1 reply; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-10 8:18 UTC (permalink / raw) To: Lucas Stach; +Cc: Thomas Petazzoni, barebox On 10.04.2015 09:23, Lucas Stach wrote: > Am Freitag, den 10.04.2015, 03:01 +0200 schrieb Sebastian Hesselbarth: >> Commit a76c62f80d95860e6c5904ab5cb91667c43f61eb >> ("net: mvneta: convert to streaming DMA ops") >> converted explicit ARM cache flushes to streaming DMA calls. >> >> However, in mvneta_send() we are not interested in the sent data buffer >> anymore. Also, in mvneta_recv() the device does not care about received >> data buffer. >> >> Remove unnecessary dma_sync_single_for_cpu() in mvneta_send() and >> dma_sync_single_for_device() in mvneta_recv(). >> >> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> > > NACK: see below. > >> --- >> Cc: barebox@lists.infradead.org >> Cc: Lucas Stach <dev@lynxeye.de> >> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> >> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >> --- >> drivers/net/mvneta.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/drivers/net/mvneta.c b/drivers/net/mvneta.c >> index 3be2ec531fb1..e1c7f15210e4 100644 >> --- a/drivers/net/mvneta.c >> +++ b/drivers/net/mvneta.c >> @@ -409,7 +409,6 @@ static int mvneta_send(struct eth_device *edev, void *data, int len) >> * the Tx port status register (PTXS). >> */ >> ret = wait_on_timeout(TRANSFER_TIMEOUT, !mvneta_pending_tx(priv)); >> - dma_sync_single_for_cpu((unsigned long)data, len, DMA_TO_DEVICE); > > This makes sure the CPU has a consistent view of memory. If something > got speculatively loaded into the cache you will write out invalid data > on the next send. > >> if (ret) { >> dev_err(&edev->dev, "transmit timeout\n"); >> return ret; >> @@ -468,9 +467,6 @@ static int mvneta_recv(struct eth_device *edev) >> rxdesc->data_size - MVNETA_MH_SIZE); >> ret = 0; >> >> - dma_sync_single_for_device((unsigned long)rxdesc->buf_phys_addr, >> - ALIGN(PKTSIZE, 8), DMA_FROM_DEVICE); >> - > The buffer is reused for the next receive operation. This isn't syncing > the data to the device, but is actually a cache invalidate to make sure > there is no pending cache writeback that may corrupt your received data. > >> recv_err: >> /* reset this and get next rx descriptor*/ >> rxdesc->data_size = 0; > > The DMA API has a notion of buffer ownership. Accessing a buffer without > transferring the ownership to the appropriate entity (cpu or device) is > illegal. Using the DMA API in a non balanced manner is skipping the > ownership transfer. Lucas, I checked the Linux DMA-API documentation again, I guess this is what you are referring to? The documents neither mention "owner" nor "balance" in any way. However, I do agree that balancing the calls makes sense as long as we cannot guarantee that receive buffers are treated read-only and transmit buffers write-only. So, the NACK is correct, please drop this patch. Sebastian _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops 2015-04-10 8:18 ` Sebastian Hesselbarth @ 2015-04-10 16:09 ` Jan Lübbe 0 siblings, 0 replies; 18+ messages in thread From: Jan Lübbe @ 2015-04-10 16:09 UTC (permalink / raw) To: barebox On Fr, 2015-04-10 at 10:18 +0200, Sebastian Hesselbarth wrote: > However, I do agree that balancing the calls makes sense as long as we > cannot guarantee that receive buffers are treated read-only and > transmit buffers write-only. > > So, the NACK is correct, please drop this patch. Lucas, do we need some barebox-specific documentation on how to use the DMA API in network drivers (because RX buffers are not always read-only)? It seems several people got confused the same way. Regards, Jan -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers 2015-04-10 1:01 [PATCH 0/4] Some fixes for PCI, mvneta, xHCI Sebastian Hesselbarth ` (2 preceding siblings ...) 2015-04-10 1:01 ` [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops Sebastian Hesselbarth @ 2015-04-10 1:01 ` Sebastian Hesselbarth 2015-04-10 7:25 ` Lucas Stach 2015-04-13 14:22 ` [PATCH v2] " Sebastian Hesselbarth 3 siblings, 2 replies; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-10 1:01 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox When working with non-coherent transfer buffers, we have to sync device and cpu for outgoing and incoming buffers. Fix the driver where non-coherent buffers are used in device context. Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> --- Cc: barebox@lists.infradead.org --- drivers/usb/host/xhci-hcd.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/host/xhci-hcd.c b/drivers/usb/host/xhci-hcd.c index c3d623e91f51..fab884229732 100644 --- a/drivers/usb/host/xhci-hcd.c +++ b/drivers/usb/host/xhci-hcd.c @@ -1026,6 +1026,8 @@ static int xhci_submit_normal(struct usb_device *udev, unsigned long pipe, /* Normal TRB */ memset(&trb, 0, sizeof(union xhci_trb)); + dma_sync_single_for_device((unsigned long)buffer, length, + DMA_TO_DEVICE); trb.event_cmd.cmd_trb = cpu_to_le64((dma_addr_t)buffer); /* FIXME: TD remainder */ trb.event_cmd.status = TRB_LEN(length) | TRB_INTR_TARGET(0); @@ -1114,6 +1116,8 @@ static int xhci_submit_control(struct usb_device *udev, unsigned long pipe, /* Data TRB */ if (length > 0) { memset(&trb, 0, sizeof(union xhci_trb)); + dma_sync_single_for_device((unsigned long)buffer, length, + DMA_TO_DEVICE); trb.event_cmd.cmd_trb = cpu_to_le64((dma_addr_t)buffer); /* FIXME: TD remainder */ trb.event_cmd.status = TRB_LEN(length) | TRB_INTR_TARGET(0); @@ -1142,6 +1146,8 @@ static int xhci_submit_control(struct usb_device *udev, unsigned long pipe, length -= EVENT_TRB_LEN(trb.event_cmd.status); else if (ret < 0) return ret; + dma_sync_single_for_cpu((unsigned long)buffer, length, + DMA_FROM_DEVICE); } ret = xhci_wait_for_event(xhci, TRB_TRANSFER, &trb); -- 2.1.0 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers 2015-04-10 1:01 ` [PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers Sebastian Hesselbarth @ 2015-04-10 7:25 ` Lucas Stach 2015-04-13 14:22 ` [PATCH v2] " Sebastian Hesselbarth 1 sibling, 0 replies; 18+ messages in thread From: Lucas Stach @ 2015-04-10 7:25 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox Am Freitag, den 10.04.2015, 03:01 +0200 schrieb Sebastian Hesselbarth: > When working with non-coherent transfer buffers, we have to sync > device and cpu for outgoing and incoming buffers. Fix the driver where > non-coherent buffers are used in device context. > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> This is not using the DMA API the way it is intended. See my last reply about balanced ownership transfer. Please make sure you take this into account for this patch. > --- > Cc: barebox@lists.infradead.org > --- > drivers/usb/host/xhci-hcd.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/usb/host/xhci-hcd.c b/drivers/usb/host/xhci-hcd.c > index c3d623e91f51..fab884229732 100644 > --- a/drivers/usb/host/xhci-hcd.c > +++ b/drivers/usb/host/xhci-hcd.c > @@ -1026,6 +1026,8 @@ static int xhci_submit_normal(struct usb_device *udev, unsigned long pipe, > > /* Normal TRB */ > memset(&trb, 0, sizeof(union xhci_trb)); > + dma_sync_single_for_device((unsigned long)buffer, length, > + DMA_TO_DEVICE); > trb.event_cmd.cmd_trb = cpu_to_le64((dma_addr_t)buffer); > /* FIXME: TD remainder */ > trb.event_cmd.status = TRB_LEN(length) | TRB_INTR_TARGET(0); > @@ -1114,6 +1116,8 @@ static int xhci_submit_control(struct usb_device *udev, unsigned long pipe, > /* Data TRB */ > if (length > 0) { > memset(&trb, 0, sizeof(union xhci_trb)); > + dma_sync_single_for_device((unsigned long)buffer, length, > + DMA_TO_DEVICE); > trb.event_cmd.cmd_trb = cpu_to_le64((dma_addr_t)buffer); > /* FIXME: TD remainder */ > trb.event_cmd.status = TRB_LEN(length) | TRB_INTR_TARGET(0); > @@ -1142,6 +1146,8 @@ static int xhci_submit_control(struct usb_device *udev, unsigned long pipe, > length -= EVENT_TRB_LEN(trb.event_cmd.status); > else if (ret < 0) > return ret; > + dma_sync_single_for_cpu((unsigned long)buffer, length, > + DMA_FROM_DEVICE); > } > > ret = xhci_wait_for_event(xhci, TRB_TRANSFER, &trb); _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers 2015-04-10 1:01 ` [PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers Sebastian Hesselbarth 2015-04-10 7:25 ` Lucas Stach @ 2015-04-13 14:22 ` Sebastian Hesselbarth 2015-04-14 18:52 ` Sascha Hauer ` (2 more replies) 1 sibling, 3 replies; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-13 14:22 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox When working with non-coherent transfer buffers, we have to sync device and cpu for outgoing and incoming buffers. Fix the driver where non-coherent buffers are used in device context. Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> --- Changelog: v1->v2: - balance dma_sync_single_foo() calls (Reported by Lucas Stach) Cc: barebox@lists.infradead.org Cc: Lucas Stach <dev@lynxeye.de> --- drivers/usb/host/xhci-hcd.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-hcd.c b/drivers/usb/host/xhci-hcd.c index c3d623e91f51..a44a1a4dffa5 100644 --- a/drivers/usb/host/xhci-hcd.c +++ b/drivers/usb/host/xhci-hcd.c @@ -1024,6 +1024,11 @@ static int xhci_submit_normal(struct usb_device *udev, unsigned long pipe, GET_SLOT_STATE(le32_to_cpu(vdev->out_ctx->slot.dev_state)), epi, vdev->in_ctx, vdev->out_ctx); + /* pass ownership of data buffer to device */ + dma_sync_single_for_device((unsigned long)buffer, length, + usb_pipein(pipe) ? + DMA_FROM_DEVICE : DMA_TO_DEVICE); + /* Normal TRB */ memset(&trb, 0, sizeof(union xhci_trb)); trb.event_cmd.cmd_trb = cpu_to_le64((dma_addr_t)buffer); @@ -1037,6 +1042,11 @@ static int xhci_submit_normal(struct usb_device *udev, unsigned long pipe, ret = xhci_wait_for_event(xhci, TRB_TRANSFER, &trb); xhci_print_trb(xhci, &trb, "Response Normal"); + /* Regain ownership of data buffer from device */ + dma_sync_single_for_cpu((unsigned long)buffer, length, + usb_pipein(pipe) ? + DMA_FROM_DEVICE : DMA_TO_DEVICE); + switch (ret) { case -COMP_SHORT_TX: udev->status = 0; @@ -1094,6 +1104,11 @@ static int xhci_submit_control(struct usb_device *udev, unsigned long pipe, return ret; } + /* Pass ownership of data buffer to device */ + dma_sync_single_for_device((unsigned long)buffer, length, + (req->requesttype & USB_DIR_IN) ? + DMA_FROM_DEVICE : DMA_TO_DEVICE); + /* Setup TRB */ memset(&trb, 0, sizeof(union xhci_trb)); trb.generic.field[0] = le16_to_cpu(req->value) << 16 | @@ -1141,11 +1156,17 @@ static int xhci_submit_control(struct usb_device *udev, unsigned long pipe, if (ret == -COMP_SHORT_TX) length -= EVENT_TRB_LEN(trb.event_cmd.status); else if (ret < 0) - return ret; + goto dma_regain; } ret = xhci_wait_for_event(xhci, TRB_TRANSFER, &trb); xhci_print_trb(xhci, &trb, "Response Status"); + +dma_regain: + /* Regain ownership of data buffer from device */ + dma_sync_single_for_cpu((unsigned long)buffer, length, + (req->requesttype & USB_DIR_IN) ? + DMA_FROM_DEVICE : DMA_TO_DEVICE); if (ret < 0) return ret; -- 2.1.0 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers 2015-04-13 14:22 ` [PATCH v2] " Sebastian Hesselbarth @ 2015-04-14 18:52 ` Sascha Hauer 2015-04-14 19:29 ` Sebastian Hesselbarth 2015-04-15 9:10 ` Lucas Stach 2015-04-15 12:06 ` Sascha Hauer 2 siblings, 1 reply; 18+ messages in thread From: Sascha Hauer @ 2015-04-14 18:52 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox On Mon, Apr 13, 2015 at 04:22:30PM +0200, Sebastian Hesselbarth wrote: > When working with non-coherent transfer buffers, we have to sync > device and cpu for outgoing and incoming buffers. Fix the driver where > non-coherent buffers are used in device context. > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> Looks ok to me now. Lucas? > --- > Changelog: > v1->v2: > - balance dma_sync_single_foo() calls (Reported by Lucas Stach) > > Cc: barebox@lists.infradead.org > Cc: Lucas Stach <dev@lynxeye.de> Seems your git send-email ignored this. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers 2015-04-14 18:52 ` Sascha Hauer @ 2015-04-14 19:29 ` Sebastian Hesselbarth 2015-04-15 9:11 ` Lucas Stach 0 siblings, 1 reply; 18+ messages in thread From: Sebastian Hesselbarth @ 2015-04-14 19:29 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox On 14.04.2015 20:52, Sascha Hauer wrote: > On Mon, Apr 13, 2015 at 04:22:30PM +0200, Sebastian Hesselbarth wrote: >> --- >> Changelog: >> v1->v2: >> - balance dma_sync_single_foo() calls (Reported by Lucas Stach) >> >> Cc: barebox@lists.infradead.org >> Cc: Lucas Stach <dev@lynxeye.de> > > Seems your git send-email ignored this. Did it? I checked the original mail's header and it says: From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> Cc: barebox@lists.infradead.org, Lucas Stach <dev@lynxeye.de> Subject: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers I also checked http://www.spinics.net/lists/u-boot-v2/msg23105.html which does not show Lucas on Cc. @Lucas, did you receive the original mail or should I worry about my git-email setup? Sebastian _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers 2015-04-14 19:29 ` Sebastian Hesselbarth @ 2015-04-15 9:11 ` Lucas Stach 0 siblings, 0 replies; 18+ messages in thread From: Lucas Stach @ 2015-04-15 9:11 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox Am Dienstag, den 14.04.2015, 21:29 +0200 schrieb Sebastian Hesselbarth: > On 14.04.2015 20:52, Sascha Hauer wrote: > > On Mon, Apr 13, 2015 at 04:22:30PM +0200, Sebastian Hesselbarth wrote: > >> --- > >> Changelog: > >> v1->v2: > >> - balance dma_sync_single_foo() calls (Reported by Lucas Stach) > >> > >> Cc: barebox@lists.infradead.org > >> Cc: Lucas Stach <dev@lynxeye.de> > > > > Seems your git send-email ignored this. > > Did it? I checked the original mail's header and it says: > > From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> > To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> > Cc: barebox@lists.infradead.org, Lucas Stach <dev@lynxeye.de> > Subject: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers > > I also checked http://www.spinics.net/lists/u-boot-v2/msg23105.html > which does not show Lucas on Cc. > > @Lucas, did you receive the original mail or should I worry about > my git-email setup? > No worries. Your E-Mail setup is just fine. The barebox mailinglist seems to eat the CC list, which is a bit annoying. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers 2015-04-13 14:22 ` [PATCH v2] " Sebastian Hesselbarth 2015-04-14 18:52 ` Sascha Hauer @ 2015-04-15 9:10 ` Lucas Stach 2015-04-15 12:06 ` Sascha Hauer 2 siblings, 0 replies; 18+ messages in thread From: Lucas Stach @ 2015-04-15 9:10 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox Am Montag, den 13.04.2015, 16:22 +0200 schrieb Sebastian Hesselbarth: > When working with non-coherent transfer buffers, we have to sync > device and cpu for outgoing and incoming buffers. Fix the driver where > non-coherent buffers are used in device context. > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > --- > Changelog: > v1->v2: > - balance dma_sync_single_foo() calls (Reported by Lucas Stach) > > Cc: barebox@lists.infradead.org > Cc: Lucas Stach <dev@lynxeye.de> > --- > drivers/usb/host/xhci-hcd.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/xhci-hcd.c b/drivers/usb/host/xhci-hcd.c > index c3d623e91f51..a44a1a4dffa5 100644 > --- a/drivers/usb/host/xhci-hcd.c > +++ b/drivers/usb/host/xhci-hcd.c > @@ -1024,6 +1024,11 @@ static int xhci_submit_normal(struct usb_device *udev, unsigned long pipe, > GET_SLOT_STATE(le32_to_cpu(vdev->out_ctx->slot.dev_state)), epi, > vdev->in_ctx, vdev->out_ctx); > > + /* pass ownership of data buffer to device */ > + dma_sync_single_for_device((unsigned long)buffer, length, > + usb_pipein(pipe) ? > + DMA_FROM_DEVICE : DMA_TO_DEVICE); > + > /* Normal TRB */ > memset(&trb, 0, sizeof(union xhci_trb)); > trb.event_cmd.cmd_trb = cpu_to_le64((dma_addr_t)buffer); > @@ -1037,6 +1042,11 @@ static int xhci_submit_normal(struct usb_device *udev, unsigned long pipe, > ret = xhci_wait_for_event(xhci, TRB_TRANSFER, &trb); > xhci_print_trb(xhci, &trb, "Response Normal"); > > + /* Regain ownership of data buffer from device */ > + dma_sync_single_for_cpu((unsigned long)buffer, length, > + usb_pipein(pipe) ? > + DMA_FROM_DEVICE : DMA_TO_DEVICE); > + > switch (ret) { > case -COMP_SHORT_TX: > udev->status = 0; > @@ -1094,6 +1104,11 @@ static int xhci_submit_control(struct usb_device *udev, unsigned long pipe, > return ret; > } > > + /* Pass ownership of data buffer to device */ > + dma_sync_single_for_device((unsigned long)buffer, length, > + (req->requesttype & USB_DIR_IN) ? > + DMA_FROM_DEVICE : DMA_TO_DEVICE); > + > /* Setup TRB */ > memset(&trb, 0, sizeof(union xhci_trb)); > trb.generic.field[0] = le16_to_cpu(req->value) << 16 | > @@ -1141,11 +1156,17 @@ static int xhci_submit_control(struct usb_device *udev, unsigned long pipe, > if (ret == -COMP_SHORT_TX) > length -= EVENT_TRB_LEN(trb.event_cmd.status); > else if (ret < 0) > - return ret; > + goto dma_regain; > } > > ret = xhci_wait_for_event(xhci, TRB_TRANSFER, &trb); > xhci_print_trb(xhci, &trb, "Response Status"); > + > +dma_regain: > + /* Regain ownership of data buffer from device */ > + dma_sync_single_for_cpu((unsigned long)buffer, length, > + (req->requesttype & USB_DIR_IN) ? > + DMA_FROM_DEVICE : DMA_TO_DEVICE); > if (ret < 0) > return ret; > -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers 2015-04-13 14:22 ` [PATCH v2] " Sebastian Hesselbarth 2015-04-14 18:52 ` Sascha Hauer 2015-04-15 9:10 ` Lucas Stach @ 2015-04-15 12:06 ` Sascha Hauer 2 siblings, 0 replies; 18+ messages in thread From: Sascha Hauer @ 2015-04-15 12:06 UTC (permalink / raw) To: Sebastian Hesselbarth; +Cc: barebox On Mon, Apr 13, 2015 at 04:22:30PM +0200, Sebastian Hesselbarth wrote: > When working with non-coherent transfer buffers, we have to sync > device and cpu for outgoing and incoming buffers. Fix the driver where > non-coherent buffers are used in device context. > > Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> > --- > Changelog: > v1->v2: > - balance dma_sync_single_foo() calls (Reported by Lucas Stach) > > Cc: barebox@lists.infradead.org > Cc: Lucas Stach <dev@lynxeye.de> > --- > drivers/usb/host/xhci-hcd.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) Applied, thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-04-15 12:07 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-10 1:01 [PATCH 0/4] Some fixes for PCI, mvneta, xHCI Sebastian Hesselbarth 2015-04-10 1:01 ` [PATCH 1/4] pci: fix device registration for directly attached devices Sebastian Hesselbarth 2015-04-10 7:15 ` Lucas Stach 2015-04-10 8:24 ` Sebastian Hesselbarth 2015-04-10 1:01 ` [PATCH 2/4] net: mvneta: Fix transmit errors due to dirty txdesc Sebastian Hesselbarth 2015-04-13 6:43 ` Sascha Hauer 2015-04-10 1:01 ` [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops Sebastian Hesselbarth 2015-04-10 7:23 ` Lucas Stach 2015-04-10 8:18 ` Sebastian Hesselbarth 2015-04-10 16:09 ` Jan Lübbe 2015-04-10 1:01 ` [PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers Sebastian Hesselbarth 2015-04-10 7:25 ` Lucas Stach 2015-04-13 14:22 ` [PATCH v2] " Sebastian Hesselbarth 2015-04-14 18:52 ` Sascha Hauer 2015-04-14 19:29 ` Sebastian Hesselbarth 2015-04-15 9:11 ` Lucas Stach 2015-04-15 9:10 ` Lucas Stach 2015-04-15 12:06 ` Sascha Hauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox