From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Mon, 9 Nov 2015 10:21:55 -0700 Subject: [U-Boot] Ethernet i210 (e1000 driver) on tegra K1 In-Reply-To: References: <56334F76.4040208@nexvision.fr> <563A544A.40007@wwwdotorg.org> Message-ID: <5640D633.6020807@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 11/04/2015 08:49 PM, Bin Meng wrote: > On Thu, Nov 5, 2015 at 2:54 AM, Stephen Warren wrote: >> On 10/30/2015 05:07 AM, Ivan Mercier wrote: >>> >>> Hi, >>> >>> I'm using a ethernet controller intel i210 >>> (http://www.commell.com.tw/product/Surveillance/MPX-210.htm) on my >>> nvidia tegra k1 jetson. >> >> >> (You didn't actually CC anyone involved with Tegra, so I only accidentally >> noticed this while I was looking at my mailing list folder) >> >>> I not an expert with pci, but the only way to make it working in u-boot >>> (upstream) is with the workaround below. >>> >>> E1000 is very common, so finding a critical bug in this driver seems >>> weird... >>> Do you think there is a bug in e1000.c or in tegra pci layer? >> >> >>> diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c >> >> >>> @@ -5186,7 +5186,7 @@ static int _e1000_transmit(struct e1000_hw *hw, >>> void *txpacket, int length) >>> txp = tx_base + tx_tail; >>> tx_tail = (tx_tail + 1) % 8; >>> >>> - txp->buffer_addr = cpu_to_le64(virt_to_bus(hw->pdev, nv_packet)); >>> + txp->buffer_addr = cpu_to_le64((unsigned long) nv_packet); >>> txp->lower.data = cpu_to_le32(hw->txd_cmd | length); >>> txp->upper.data = 0; >> >> >> In order to work out what's going on, perhaps you could print out the values >> of nv_packet and virt_to_bus(hw->pdev, nv_packet). >> >> It's not terribly surprising that removing the call to virt_to_bus works, >> since IIRC U-Boot on Tegra uses the same address setup for the PCIe bus as >> for CPU physical addresses as for CPU virtual addresses. >> >> So, the question is: what is virt_to_bus() doing, and is it the right API to >> call? > > virt_to_bus() is to translate cpu virtual address to pci bus physical > address. Basically two levels of translation. > >> I see that virt_to_bus() is defined as: >> >> e1000.c: >> >> #define virt_to_bus(devno, v) pci_virt_to_mem(devno, (void *) (v)) >> >> pci.h: >> >> #define pci_virt_to_mem(dev, addr) \ >> pci_virt_to_bus((dev), (addr), PCI_REGION_MEM) >> >> #define pci_virt_to_bus(dev, addr, flags) \ >> pci_hose_phys_to_bus(pci_bus_to_hose(PCI_BUS(dev)), \ >> (virt_to_phys(addr)), (flags)) >> >> I know that the RTL8169 driver works on the same board (it's soldered down >> and attached to the other PCIe root port on the SoC). For what looks like >> the same "use case", it seems to call pci_mem_to_phys() which is: >> >> pci.h: >> >> #define pci_mem_to_phys(dev, addr) \ >> pci_bus_to_phys((dev), (addr), PCI_REGION_MEM) >> >> #define pci_bus_to_phys(dev, addr, flags) \ >> pci_hose_bus_to_phys(pci_bus_to_hose(PCI_BUS(dev)), (addr), \ >> (flags)) >> >> That's odd, since one of those does a phys -> bus translation and the other >> does a bus -> phys translation. That's the opposite direction, so both can't >> possibly be right. I wonder if those mapping functions are no-ops on > > Indeed these address translation macros in pci.h are confusing. But > most SoC defines 1:1:1 mapping so these are not needed. The reason why > it fails on Tegra is probably due to the addresses are not 1:1:1 > mapped? I'm pretty sure everything is 1:1. That's why removing the call to any translation macro made the e1000 driver work for Ivan.