diff mbox

[U-Boot,v2,01/40] vsprintf: Add modifier for phys_addr_t

Message ID 20140828103802.GC14388@ulmo
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Thierry Reding Aug. 28, 2014, 10:38 a.m. UTC 
On Wed, Aug 27, 2014 at 11:41:55AM -0600, Stephen Warren wrote:
> On 08/27/2014 01:01 AM, Thierry Reding wrote:
> >On Tue, Aug 26, 2014 at 11:04:56AM -0600, Stephen Warren wrote:
> >>On 08/26/2014 09:33 AM, Thierry Reding wrote:
> >>>From: Thierry Reding <treding@nvidia.com>
> >>>
> >>>Provide a new modifier to vsprintf() to print phys_addr_t variables to
> >>>avoid having to cast or #ifdef when printing them out. The %pa modifier
> >>>is used for this purpose, so phys_addr_t variables need to be passed by
> >>>reference, like so:
> >>>
> >>>	phys_addr_t start = 0;
> >>>
> >>>	printf("start: %pa\n", &start);
> >>>
> >>>Depending on the size of phys_addr_t this will print out the address
> >>>with 8 or 16 hexadecimal digits following a 0x prefix.
> >>
> >>The series,
> >>
> >>Tested-by: Stephen Warren <swarren@nvidia.com>
> >>
> >>Note that I did see the following printed a couple of times when I executed
> >>"run bootcmd_pxe":
> >>
> >>pci_hose_bus_to_phys: invalid physical address
> >>
> >>... but everything worked perfectly, so I guess we can track that down
> >>later.
> >
> >Yes, it should definitely be tracked down. I don't see that message on
> >my setup. I've seen it for example when noncached_alloc() fails and
> >returns 0, but in that case everything shouldn't be working perfectly.
> 
> It looks like it happens when "dhcp" or "pxe get" attempt to download a file
> that doesn't exist on the server.
> 
> >It would be helpful if that message showed what physical address was
> >considered invalid.
> 
> The address is 0. I think this is because rtl_recv() is being called after
> rtl_halt(). rtl_halt() NULLs out all tpc->RxBufferRing[] entries, so calling
> rtl_recv() after that tries to convert address 0 to a PCI address. I haven't
> fully tracked this down, but it seems like a bug in the U-Boot networking or
> TFTP core, rather than anything to do with drivers for R8169 or the Tegra
> PCIe controller.
> 
> I'm not planning on tracking this down any further at the moment.

The attached patch fixes this issue for me. Cc'ing Joe to help figure
out if that's the right approach.

Thierry

Comments

Tom Rini Sept. 17, 2014, 12:44 a.m. UTC | #1 
> From: Thierry Reding <treding@nvidia.com>
> Date: Thu, 28 Aug 2014 12:26:58 +0200
> Subject: [PATCH] rtl8169: Defer network packet processing
> 
> When network protocol errors occur (such as a file not being found on a
> TFTP server), the processing done by the NetReceive() function will end
> up calling the driver's .halt() implementation. However, after that the
> device no longer has access to the memory buffers and will cause errors
> such as this in the rtl_recv() function when trying to hand descriptors
> back to the device:
> 
> 	pci_hose_bus_to_phys: invalid physical address
> 
> This can be fixed by deferring processing of network packets until the
> descriptors have been handed back. That way rtl_halt() tearing down
> network buffers is not going to prevent access to the buffers.
> 
> Reported-by: Stephen Warren <swarren@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Applied to u-boot/master, thanks!
diff mbox

Patch

From 89f5c6e725cbce1fa7ff4f69c3c8efae89b629de Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@nvidia.com>
Date: Thu, 28 Aug 2014 12:26:58 +0200
Subject: [PATCH] rtl8169: Defer network packet processing

When network protocol errors occur (such as a file not being found on a
TFTP server), the processing done by the NetReceive() function will end
up calling the driver's .halt() implementation. However, after that the
device no longer has access to the memory buffers and will cause errors
such as this in the rtl_recv() function when trying to hand descriptors
back to the device:

	pci_hose_bus_to_phys: invalid physical address

This can be fixed by deferring processing of network packets until the
descriptors have been handed back. That way rtl_halt() tearing down
network buffers is not going to prevent access to the buffers.

Reported-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/rtl8169.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index 8183df39e77a..a722fcf9c752 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -527,7 +527,6 @@  static int rtl_recv(struct eth_device *dev)
 
 			rtl_inval_buffer(tpc->RxBufferRing[cur_rx], length);
 			memcpy(rxdata, tpc->RxBufferRing[cur_rx], length);
-			NetReceive(rxdata, length);
 
 			if (cur_rx == NUM_RX_DESC - 1)
 				tpc->RxDescArray[cur_rx].status =
@@ -538,6 +537,8 @@  static int rtl_recv(struct eth_device *dev)
 			tpc->RxDescArray[cur_rx].buf_addr =
 				cpu_to_le32(bus_to_phys(tpc->RxBufferRing[cur_rx]));
 			rtl_flush_rx_desc(&tpc->RxDescArray[cur_rx]);
+
+			NetReceive(rxdata, length);
 		} else {
 			puts("Error Rx");
 		}
-- 
2.0.4