[PATCH 0/4] Some fixes for PCI, mvneta, xHCI

[PATCH 0/4] Some fixes for PCI, mvneta, xHCI

[Sebastian Hesselbarth]

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

[PATCH 1/4] pci: fix device registration for directly attached devices

[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>
---
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

Re: [PATCH 1/4] pci: fix device registration for directly attached devices

[Lucas Stach]

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

Re: [PATCH 1/4] pci: fix device registration for directly attached devices

[Sebastian Hesselbarth]

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

[PATCH 2/4] net: mvneta: Fix transmit errors due to dirty txdesc

[Sebastian Hesselbarth]

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

Re: [PATCH 2/4] net: mvneta: Fix transmit errors due to dirty txdesc

[Sascha Hauer]

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

[PATCH 3/4] net: mvneta: Remove unnecessary DMA ops

[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>
---
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

Re: [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops

[Lucas Stach]

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

Re: [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops

[Sebastian Hesselbarth]

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

Re: [PATCH 3/4] net: mvneta: Remove unnecessary DMA ops

[Jan Lübbe]

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

[PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers

[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>
---
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

Re: [PATCH 4/4] USB: xHCI: Sync non-coherent DMA buffers

[Lucas Stach]

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

[PATCH v2] USB: xHCI: Sync non-coherent DMA buffers

[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>
---
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

Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers

[Sascha Hauer]

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

Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers

[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?

Sebastian

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers

[Lucas Stach]

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

Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers

[Lucas Stach]

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

Re: [PATCH v2] USB: xHCI: Sync non-coherent DMA buffers

[Sascha Hauer]

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