On Thu, Jun 13, 2019 at 03:16:10PM +0800, Bin Meng wrote:
Hi Stefan,
On Thu, Jun 13, 2019 at 1:40 PM Stefan Roese sr@denx.de wrote:
Added Bin, Joe and Thierry to Cc
On 11.06.19 13:15, Patrick Wildt wrote:
Hi,
I have an rtl8169 on a macchiatobin and that card has a 64-bit memory address. The current code only reads a single word, which means it can only support a 32-bit address. By using dm_pci_map_bar we don't need to manually parse the register, we can just have it do its job.
I'm not sure though if this works for all devices since the previous version had an explicit check for the device.
Patrick
Signed-off-by: Patrick Wildt patrick@blueri.se
diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c index 521e5909a2..f1d2ade253 100644 --- a/drivers/net/rtl8169.c +++ b/drivers/net/rtl8169.c @@ -1182,22 +1182,11 @@ static int rtl8169_eth_probe(struct udevice *dev) struct pci_child_platdata *pplat = dev_get_parent_platdata(dev); struct rtl8169_private *priv = dev_get_priv(dev); struct eth_pdata *plat = dev_get_platdata(dev);
u32 iobase;int region; int ret;debug("rtl8169: REALTEK RTL8169 @0x%x\n", iobase);switch (pplat->device) {case 0x8168:region = 2;break;default:region = 1;break;}dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0 + region * 4, &iobase);iobase &= ~0xf;priv->iobase = (int)dm_pci_mem_to_phys(dev, iobase);
priv->iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2,PCI_REGION_MEM);printf("rtl8169: REALTEK RTL8169 @0x%x\n", priv->iobase); ret = rtl_init(priv->iobase, dev->name, plat->enetaddr); if (ret < 0) {Bin, Joe, Thierry,
do you have any comments on this patch? Moving unconditionally to one BAR instead of BAR1/2 depending on the chip version seems a bit "brave".
Agreed that blinding setting one BAR for the iobase is not a good idea.
Agreed. I don't know whether it's actually required to differentiate based on version, but I suppose the code is like that for a reason, so better keep that.
Also, looking at dm_pci_map_bar() it doesn't look like that does anything different than the existing code. It merely reads a single 32-bit register, so it doesn't properly deal with 64-bit BARs either.
I suppose that could be fixed in dm_pci_map_bar(), and then the fix would automatically propagate to all users of that, which is good. But I don't think it currently works correctly.
Thierry