[U-Boot] Ethernet i210 (e1000 driver) on tegra K1

[Stephen Warren]

On 11/04/2015 08:49 PM, Bin Meng wrote:
> On Thu, Nov 5, 2015 at 2:54 AM, Stephen Warren <swarren at wwwdotorg.org> 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.