| Message ID | 20190510025611.4296-1-marek.behun@nic.cz |
|---|---|
| State | Deferred |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [U-Boot,1/1] pci: pci_mvebu: ignore the local device | expand |
Hi Marek, On Fri, May 10, 2019 at 2:56 PM Marek Behún <marek.behun@nic.cz> wrote: > > The local device (host "bridge"?) on pci_mvebu controller reports > PCI_CLASS_MEMORY_OTHER in the class register. > > This does not work in U-Boot, because U-Boot then tries to autoconfigure > this device in pci_auto.c. This causes the real connected PCIe device > not to work (in U-Boot nor in Linux). > > Linux solves this by emulating PCI bridge device (see > drivers/pci/pci-bridge-emul.c in Linux). The Marvell vendor/device IDs > are used for this pci-bridge-emul device in Linux, but the actual > register accesses are emulated and the value of class register is > PCI_CLASS_BRIDGE_PCI. > > The simplest solution here in my opinion is to just ignore the local > device in this driver's read/write methods. This fixes PCIe issues for > me. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Cc: Stefan Roese <sr@denx.de> > Cc: Anton Schubert <anton.schubert@gmx.de> > Cc: Dirk Eibach <dirk.eibach@gdsys.cc> > Cc: Mario Six <mario.six@gdsys.cc> > Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> > Cc: Phil Sutter <phil@nwl.cc> > Cc: VlaoMao <vlaomao@gmail.com> On the Allied Telesis x530 before: => pci enum => pci Scanning PCI devices on bus 0 BusDevFun VendorId DeviceId Device Class Sub-Class
On 10.05.19 04:56, Marek Behún wrote: > The local device (host "bridge"?) on pci_mvebu controller reports > PCI_CLASS_MEMORY_OTHER in the class register. > > This does not work in U-Boot, because U-Boot then tries to autoconfigure > this device in pci_auto.c. This causes the real connected PCIe device > not to work (in U-Boot nor in Linux). What exactly does not work or is the issue here with the current implementation in Linux? I'm asking, since we have not noticed any issues here? Thanks, Stefan > Linux solves this by emulating PCI bridge device (see > drivers/pci/pci-bridge-emul.c in Linux). The Marvell vendor/device IDs > are used for this pci-bridge-emul device in Linux, but the actual > register accesses are emulated and the value of class register is > PCI_CLASS_BRIDGE_PCI. > > The simplest solution here in my opinion is to just ignore the local > device in this driver's read/write methods. This fixes PCIe issues for > me. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Cc: Stefan Roese <sr@denx.de> > Cc: Anton Schubert <anton.schubert@gmx.de> > Cc: Dirk Eibach <dirk.eibach@gdsys.cc> > Cc: Mario Six <mario.six@gdsys.cc> > Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> > Cc: Phil Sutter <phil@nwl.cc> > Cc: VlaoMao <vlaomao@gmail.com> > --- > drivers/pci/pci_mvebu.c | 59 +++++++++++++++++------------------------ > 1 file changed, 24 insertions(+), 35 deletions(-) > > diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c > index e21dc10c2f..653f445a0f 100644 > --- a/drivers/pci/pci_mvebu.c > +++ b/drivers/pci/pci_mvebu.c > @@ -143,31 +143,31 @@ static int mvebu_pcie_read_config(struct udevice *bus, pci_dev_t bdf, > struct mvebu_pcie *pcie = dev_get_platdata(bus); > int local_bus = PCI_BUS(pcie->dev); > int local_dev = PCI_DEV(pcie->dev); > + int other_dev; > u32 reg; > u32 data; > > debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ", > PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); > > - /* Only allow one other device besides the local one on the local bus */ > - if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) { > - if (local_dev == 0 && PCI_DEV(bdf) != 1) { > - debug("- out of range\n"); > - /* > - * If local dev is 0, the first other dev can > - * only be 1 > - */ > - *valuep = pci_get_ff(size); > - return 0; > - } else if (local_dev != 0 && PCI_DEV(bdf) != 0) { > - debug("- out of range\n"); > - /* > - * If local dev is not 0, the first other dev can > - * only be 0 > - */ > - *valuep = pci_get_ff(size); > - return 0; > - } > + /* > + * The local device has PCI_CLASS_MEMORY_OTHER in the class register. > + * This does not work for U-Boot because pci_auto.c tries to configure > + * this as a device. This results in U-Boot/Linux being unable to access > + * PCIe devices. > + * > + * Linux solves this by emulating a bridge (see > + * drivers/pci/pci-bridge-emul.c in Linux). > + * > + * Let's ignore the local device in U-Boot. > + * > + * Also only allow one other device besides the (ignored) local one. > + */ > + > + other_dev = local_dev ? 0 : 1; > + if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != other_dev) { > + *valuep = pci_get_ff(size); > + return 0; > } > > /* write address */ > @@ -187,28 +187,17 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf, > struct mvebu_pcie *pcie = dev_get_platdata(bus); > int local_bus = PCI_BUS(pcie->dev); > int local_dev = PCI_DEV(pcie->dev); > + int other_dev; > u32 data; > > debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ", > PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); > debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value); > > - /* Only allow one other device besides the local one on the local bus */ > - if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) { > - if (local_dev == 0 && PCI_DEV(bdf) != 1) { > - /* > - * If local dev is 0, the first other dev can > - * only be 1 > - */ > - return 0; > - } else if (local_dev != 0 && PCI_DEV(bdf) != 0) { > - /* > - * If local dev is not 0, the first other dev can > - * only be 0 > - */ > - return 0; > - } > - } > + /* See the comment in mvebu_pcie_read_config */ > + other_dev = local_dev ? 0 : 1; > + if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != other_dev) > + return 0; > > writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF); > data = pci_conv_size_to_32(0, value, offset, size); > Viele Grüße, Stefan
On Fri, 10 May 2019 10:15:25 +0200 Stefan Roese <sr@denx.de> wrote: > On 10.05.19 04:56, Marek Behún wrote: > > The local device (host "bridge"?) on pci_mvebu controller reports > > PCI_CLASS_MEMORY_OTHER in the class register. > > > > This does not work in U-Boot, because U-Boot then tries to autoconfigure > > this device in pci_auto.c. This causes the real connected PCIe device > > not to work (in U-Boot nor in Linux). > > What exactly does not work or is the issue here with the current > implementation in Linux? I'm asking, since we have not noticed any > issues here? > ath10k firmware fails to load for network device. SATA device timeouts on a PCIe SATA expander. As if some PCIe I/O did not work. The devices are still enumerated and recognized by Linux, but simply won't work. Nor in U-Boot.
On 10.05.19 13:44, Marek Behun wrote: > On Fri, 10 May 2019 10:15:25 +0200 > Stefan Roese <sr@denx.de> wrote: > >> On 10.05.19 04:56, Marek Behún wrote: >>> The local device (host "bridge"?) on pci_mvebu controller reports >>> PCI_CLASS_MEMORY_OTHER in the class register. >>> >>> This does not work in U-Boot, because U-Boot then tries to autoconfigure >>> this device in pci_auto.c. This causes the real connected PCIe device >>> not to work (in U-Boot nor in Linux). >> >> What exactly does not work or is the issue here with the current >> implementation in Linux? I'm asking, since we have not noticed any >> issues here? >> > ath10k firmware fails to load for network device. > SATA device timeouts on a PCIe SATA expander. > As if some PCIe I/O did not work. The devices are still enumerated and > recognized by Linux, but simply won't work. Nor in U-Boot. I see. Strange that we did not see any issues on our board (or other users). I'll try to pull this patch into this upcoming release though. Thanks, Stefan
Hi Marek, On Fri, May 10, 2019 at 11:44 PM Marek Behun <marek.behun@nic.cz> wrote: > > On Fri, 10 May 2019 10:15:25 +0200 > Stefan Roese <sr@denx.de> wrote: > > > On 10.05.19 04:56, Marek Behún wrote: > > > The local device (host "bridge"?) on pci_mvebu controller reports > > > PCI_CLASS_MEMORY_OTHER in the class register. > > > > > > This does not work in U-Boot, because U-Boot then tries to autoconfigure > > > this device in pci_auto.c. This causes the real connected PCIe device > > > not to work (in U-Boot nor in Linux). > > > > What exactly does not work or is the issue here with the current > > implementation in Linux? I'm asking, since we have not noticed any > > issues here? > > > ath10k firmware fails to load for network device. > SATA device timeouts on a PCIe SATA expander. > As if some PCIe I/O did not work. The devices are still enumerated and > recognized by Linux, but simply won't work. Nor in U-Boot. Is there a PCIe bridge involved in either of these? In the pre-history of Kirkwood (which I believe has the same or similar IP block for PCIe) I've hit platforms where the PCIe I/O fails because something about the host bridge interferes with a real bridge on the bus where a directly connected device worked fine. Marvell were shipping a patch that adds a fake PCIe bridge in their LSP and I think that is what was eventually ported to upstream Linux.
> Is there a PCIe bridge involved in either of these? In the pre-history > of Kirkwood (which I believe has the same or similar IP block for > PCIe) I've hit platforms where the PCIe I/O fails because something > about the host bridge interferes with a real bridge on the bus where a > directly connected device worked fine. Marvell were shipping a patch > that adds a fake PCIe bridge in their LSP and I think that is what was > eventually ported to upstream Linux. Hi Chris, there is no real bridge on Turris Omnia, the devices are connected directly to CPU. Marek
diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index e21dc10c2f..653f445a0f 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -143,31 +143,31 @@ static int mvebu_pcie_read_config(struct udevice *bus, pci_dev_t bdf, struct mvebu_pcie *pcie = dev_get_platdata(bus); int local_bus = PCI_BUS(pcie->dev); int local_dev = PCI_DEV(pcie->dev); + int other_dev; u32 reg; u32 data; debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ", PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); - /* Only allow one other device besides the local one on the local bus */ - if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) { - if (local_dev == 0 && PCI_DEV(bdf) != 1) { - debug("- out of range\n"); - /* - * If local dev is 0, the first other dev can - * only be 1 - */ - *valuep = pci_get_ff(size); - return 0; - } else if (local_dev != 0 && PCI_DEV(bdf) != 0) { - debug("- out of range\n"); - /* - * If local dev is not 0, the first other dev can - * only be 0 - */ - *valuep = pci_get_ff(size); - return 0; - } + /* + * The local device has PCI_CLASS_MEMORY_OTHER in the class register. + * This does not work for U-Boot because pci_auto.c tries to configure + * this as a device. This results in U-Boot/Linux being unable to access + * PCIe devices. + * + * Linux solves this by emulating a bridge (see + * drivers/pci/pci-bridge-emul.c in Linux). + * + * Let's ignore the local device in U-Boot. + * + * Also only allow one other device besides the (ignored) local one. + */ + + other_dev = local_dev ? 0 : 1; + if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != other_dev) { + *valuep = pci_get_ff(size); + return 0; } /* write address */ @@ -187,28 +187,17 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf, struct mvebu_pcie *pcie = dev_get_platdata(bus); int local_bus = PCI_BUS(pcie->dev); int local_dev = PCI_DEV(pcie->dev); + int other_dev; u32 data; debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ", PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value); - /* Only allow one other device besides the local one on the local bus */ - if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) { - if (local_dev == 0 && PCI_DEV(bdf) != 1) { - /* - * If local dev is 0, the first other dev can - * only be 1 - */ - return 0; - } else if (local_dev != 0 && PCI_DEV(bdf) != 0) { - /* - * If local dev is not 0, the first other dev can - * only be 0 - */ - return 0; - } - } + /* See the comment in mvebu_pcie_read_config */ + other_dev = local_dev ? 0 : 1; + if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != other_dev) + return 0; writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF); data = pci_conv_size_to_32(0, value, offset, size);
The local device (host "bridge"?) on pci_mvebu controller reports PCI_CLASS_MEMORY_OTHER in the class register. This does not work in U-Boot, because U-Boot then tries to autoconfigure this device in pci_auto.c. This causes the real connected PCIe device not to work (in U-Boot nor in Linux). Linux solves this by emulating PCI bridge device (see drivers/pci/pci-bridge-emul.c in Linux). The Marvell vendor/device IDs are used for this pci-bridge-emul device in Linux, but the actual register accesses are emulated and the value of class register is PCI_CLASS_BRIDGE_PCI. The simplest solution here in my opinion is to just ignore the local device in this driver's read/write methods. This fixes PCIe issues for me. Signed-off-by: Marek Behún <marek.behun@nic.cz> Cc: Stefan Roese <sr@denx.de> Cc: Anton Schubert <anton.schubert@gmx.de> Cc: Dirk Eibach <dirk.eibach@gdsys.cc> Cc: Mario Six <mario.six@gdsys.cc> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> Cc: Phil Sutter <phil@nwl.cc> Cc: VlaoMao <vlaomao@gmail.com> --- drivers/pci/pci_mvebu.c | 59 +++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 35 deletions(-)