[U-Boot] rtl8169: use dm_pci_map_bar
Thierry Reding
thierry.reding at gmail.com
Fri Jun 14 09:55:56 UTC 2019
On Fri, Jun 14, 2019 at 01:16:32AM +0200, Patrick Wildt wrote:
> On Thu, Jun 13, 2019 at 06:43:25PM +0200, Thierry Reding wrote:
> > 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 at 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 at 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
>
> Oh, I'm very sorry to have wasted your time. Yes, I agree, the
> differentiation must come back.
>
> To be honest it was about half a year ago, so I think I forgot the
> actual issue. Maybe dm_pci_mem_to_phys() returned 0 for whatever
> reason, but I don't remember. The address is 32-bit, 0xf6000000,
> so it definitely wasn't the 64-bit think I rambled about. Sorry.
>
> I will try to find out what was wrong and report back.
>
> By the way, the debug() line in the current code prints iobase which at
> that time isn't even initialized.
Yeah, that's definitely a bug. I think that warrants a separate patch.
Mind sending out a patch with just that change?
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190614/7859aac6/attachment-0001.sig>
More information about the U-Boot
mailing list