[PATCH 0/3] A minor DS414 update
In another attempt at making the two rear USB ports usable I spent a few free days to play around with PCI and XHCI. The USB controller is detected and "seems" to work, but only as long as no device is plugged in. If so, slot enable (TRB_ENABLE_SLOT) command times out and I can't find out why.
This series collects the brighter bits: Apart from a rather unrelated review of include/configs/ds414.h in patch one, patches two and three at least fix for the error messages emitted by 'pci enum' trying to PNP devices.
Phil Sutter (3): arm: mvebu: ds414: Config header mini-review pci: Make auto-config code a little more robust pci: pci_mvebu: Define an IO region as well
drivers/pci/pci_auto.c | 9 +++++---- drivers/pci/pci_mvebu.c | 28 +++++++++++++++++++++++++++- include/configs/ds414.h | 21 +++------------------ 3 files changed, 35 insertions(+), 23 deletions(-)
A few minor changes:
* Get rid of leftover comments, other commits removed the defines they referred to.
* CONFIG_SYS_NETA_INTERFACE_TYPE is not used anymore since commit e3b9c98a23ca9 ("net: mvneta: Convert to driver model").
* Drop CONFIG_USB_MAX_CONTROLLER_COUNT: it is per-HCI type, so XHCI and EHCI could still both work be used.
* Unconditionally define CONFIG_EHCI_IS_TDI: it has no effect on XHCI so that conditional doesn't make any sense.
* Define a larger PHY_ANEG_TIMEOUT: In my test bed, the NIC is directly connected to some RTL8111 and the default 8s timeout was often too short.
Signed-off-by: Phil Sutter phil@nwl.cc --- include/configs/ds414.h | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/include/configs/ds414.h b/include/configs/ds414.h index a92e788a38b9e..8aa2d47bec68d 100644 --- a/include/configs/ds414.h +++ b/include/configs/ds414.h @@ -24,31 +24,13 @@ #define CONFIG_SYS_I2C_SLAVE 0x0 #define CONFIG_SYS_I2C_SPEED 100000
-/* Environment in SPI NOR flash */ - -#define CONFIG_SYS_NETA_INTERFACE_TYPE PHY_INTERFACE_MODE_RGMII - /* PCIe support */ #ifndef CONFIG_SPL_BUILD #define CONFIG_PCI_SCAN_SHOW #endif
/* USB/EHCI/XHCI configuration */ - -#define CONFIG_USB_MAX_CONTROLLER_COUNT 2 - -/* FIXME: broken XHCI support - * Below defines should enable support for the two rear USB3 ports. Sadly, this - * does not work because: - * - xhci-pci seems to not support DM_USB, so with that enabled it is not - * found. - * - USB init fails, controller does not respond in time */ - -#if !defined(CONFIG_USB_XHCI_HCD) #define CONFIG_EHCI_IS_TDI -#endif - -/* why is this only defined in mv-common.h if CONFIG_DM is undefined? */
/* * mv-common.h should be defined after CMD configs since it used them @@ -95,4 +77,7 @@ #define CONFIG_BOOTCOMMAND "sf read ${loadaddr} 0xd0000 0x700000; bootm" #define CONFIG_LOADADDR 0x80000
+/* increase autoneg timeout, my NIC sucks */ +#define PHY_ANEG_TIMEOUT 16000 + #endif /* _CONFIG_SYNOLOGY_DS414_H */
On 03.01.21 23:06, Phil Sutter wrote:
A few minor changes:
Get rid of leftover comments, other commits removed the defines they referred to.
CONFIG_SYS_NETA_INTERFACE_TYPE is not used anymore since commit e3b9c98a23ca9 ("net: mvneta: Convert to driver model").
Drop CONFIG_USB_MAX_CONTROLLER_COUNT: it is per-HCI type, so XHCI and EHCI could still both work be used.
Unconditionally define CONFIG_EHCI_IS_TDI: it has no effect on XHCI so that conditional doesn't make any sense.
Define a larger PHY_ANEG_TIMEOUT: In my test bed, the NIC is directly connected to some RTL8111 and the default 8s timeout was often too short.
Signed-off-by: Phil Sutter phil@nwl.cc
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
include/configs/ds414.h | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/include/configs/ds414.h b/include/configs/ds414.h index a92e788a38b9e..8aa2d47bec68d 100644 --- a/include/configs/ds414.h +++ b/include/configs/ds414.h @@ -24,31 +24,13 @@ #define CONFIG_SYS_I2C_SLAVE 0x0 #define CONFIG_SYS_I2C_SPEED 100000
-/* Environment in SPI NOR flash */
-#define CONFIG_SYS_NETA_INTERFACE_TYPE PHY_INTERFACE_MODE_RGMII
/* PCIe support */ #ifndef CONFIG_SPL_BUILD #define CONFIG_PCI_SCAN_SHOW #endif
/* USB/EHCI/XHCI configuration */
-#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
-/* FIXME: broken XHCI support
- Below defines should enable support for the two rear USB3 ports. Sadly, this
- does not work because:
- xhci-pci seems to not support DM_USB, so with that enabled it is not
- found.
- USB init fails, controller does not respond in time */
-#if !defined(CONFIG_USB_XHCI_HCD) #define CONFIG_EHCI_IS_TDI -#endif
-/* why is this only defined in mv-common.h if CONFIG_DM is undefined? */
/*
- mv-common.h should be defined after CMD configs since it used them
@@ -95,4 +77,7 @@ #define CONFIG_BOOTCOMMAND "sf read ${loadaddr} 0xd0000 0x700000; bootm" #define CONFIG_LOADADDR 0x80000
+/* increase autoneg timeout, my NIC sucks */ +#define PHY_ANEG_TIMEOUT 16000
- #endif /* _CONFIG_SYNOLOGY_DS414_H */
Viele Grüße, Stefan
On my DS414, some PCI devices return odd values when probing BAR sizes. An obvious case is all-ones response, the Linux driver (drivers/pci/probe.c) catches those explicitly and a comment explains that either bit 0 or bit 1 must be clear (depending on MEM or IO type). Other BARs return e.g. 0xfff0000f or 0xfff00004 and thus manage to break size calculation due to the "middle" zeroes. Mitigate that copying more or less what Linux does and do a "find least bit set".
Signed-off-by: Phil Sutter phil@nwl.cc --- drivers/pci/pci_auto.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index 3f46b7697d7ca..ea202b0e0959e 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -47,16 +47,17 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num, dm_pci_write_config32(dev, bar, 0xffffffff); dm_pci_read_config32(dev, bar, &bar_response);
- /* If BAR is not implemented go to the next BAR */ - if (!bar_response) + /* If BAR is not implemented (or invalid) go to the next BAR */ + if (!bar_response || bar_response == 0xffffffff) continue;
found_mem64 = 0;
/* Check the BAR type and set our address mask */ if (bar_response & PCI_BASE_ADDRESS_SPACE) { - bar_size = ((~(bar_response & PCI_BASE_ADDRESS_IO_MASK)) - & 0xffff) + 1; + bar_size = bar_response & PCI_BASE_ADDRESS_IO_MASK; + bar_size &= ~(bar_size - 1); + if (!enum_only) bar_res = io;
Adding a few more PCI experts (Simon & Bin) to Cc.
On 03.01.21 23:06, Phil Sutter wrote:
On my DS414, some PCI devices return odd values when probing BAR sizes. An obvious case is all-ones response, the Linux driver (drivers/pci/probe.c) catches those explicitly and a comment explains that either bit 0 or bit 1 must be clear (depending on MEM or IO type). Other BARs return e.g. 0xfff0000f or 0xfff00004 and thus manage to break size calculation due to the "middle" zeroes. Mitigate that copying more or less what Linux does and do a "find least bit set".
Signed-off-by: Phil Sutter phil@nwl.cc
drivers/pci/pci_auto.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index 3f46b7697d7ca..ea202b0e0959e 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -47,16 +47,17 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num, dm_pci_write_config32(dev, bar, 0xffffffff); dm_pci_read_config32(dev, bar, &bar_response);
/* If BAR is not implemented go to the next BAR */if (!bar_response)
/* If BAR is not implemented (or invalid) go to the next BAR */if (!bar_response || bar_response == 0xffffffff) continue;found_mem64 = 0;
/* Check the BAR type and set our address mask */ if (bar_response & PCI_BASE_ADDRESS_SPACE) {
bar_size = ((~(bar_response & PCI_BASE_ADDRESS_IO_MASK))& 0xffff) + 1;
bar_size = bar_response & PCI_BASE_ADDRESS_IO_MASK;bar_size &= ~(bar_size - 1);if (!enum_only) bar_res = io;
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
On Sun, 3 Jan 2021 at 15:05, Phil Sutter phil@nwl.cc wrote:
On my DS414, some PCI devices return odd values when probing BAR sizes. An obvious case is all-ones response, the Linux driver (drivers/pci/probe.c) catches those explicitly and a comment explains that either bit 0 or bit 1 must be clear (depending on MEM or IO type). Other BARs return e.g. 0xfff0000f or 0xfff00004 and thus manage to break size calculation due to the "middle" zeroes. Mitigate that copying more or less what Linux does and do a "find least bit set".
Signed-off-by: Phil Sutter phil@nwl.cc
drivers/pci/pci_auto.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Configure an IO region and window for PNP identical to how MEM region is set up. Linux does this only if the DT defines a pcie-io-aperture property for the SOC, but since all supported boards do this should not be needed.
Signed-off-by: Phil Sutter phil@nwl.cc --- drivers/pci/pci_mvebu.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index c9afe7515014d..9528e7c40ecfe 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -73,6 +73,7 @@ struct mvebu_pcie { void __iomem *membase; struct resource mem; void __iomem *iobase; + struct resource io; u32 port; u32 lane; int devfn; @@ -81,6 +82,8 @@ struct mvebu_pcie { char name[16]; unsigned int mem_target; unsigned int mem_attr; + unsigned int io_target; + unsigned int io_attr; };
/* @@ -90,6 +93,7 @@ struct mvebu_pcie { */ static void __iomem *mvebu_pcie_membase = (void __iomem *)MBUS_PCI_MEM_BASE; #define PCIE_MEM_SIZE (128 << 20) +static void __iomem *mvebu_pcie_iobase = (void __iomem *)MBUS_PCI_IO_BASE;
static inline bool mvebu_pcie_link_up(struct mvebu_pcie *pcie) { @@ -306,12 +310,24 @@ static int mvebu_pcie_probe(struct udevice *dev) (u32)pcie->mem.start, PCIE_MEM_SIZE); }
+ pcie->io.start = (u32)mvebu_pcie_iobase; + pcie->io.end = pcie->io.start + MBUS_PCI_IO_SIZE - 1; + mvebu_pcie_iobase += MBUS_PCI_IO_SIZE; + + if (mvebu_mbus_add_window_by_id(pcie->io_target, pcie->io_attr, + (phys_addr_t)pcie->io.start, + MBUS_PCI_IO_SIZE)) { + printf("PCIe unable to add mbus window for IO at %08x+%08x\n", + (u32)pcie->io.start, MBUS_PCI_IO_SIZE); + } + /* Setup windows and configure host bridge */ mvebu_pcie_setup_wins(pcie);
/* Master + slave enable. */ reg = readl(pcie->base + PCIE_CMD_OFF); reg |= PCI_COMMAND_MEMORY; + reg |= PCI_COMMAND_IO; reg |= PCI_COMMAND_MASTER; reg |= BIT(10); /* disable interrupts */ writel(reg, pcie->base + PCIE_CMD_OFF); @@ -323,7 +339,9 @@ static int mvebu_pcie_probe(struct udevice *dev) 0, 0, gd->ram_size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY); - hose->region_count = 2; + pci_set_region(hose->regions + 2, pcie->io.start, + pcie->io.start, MBUS_PCI_IO_SIZE, PCI_REGION_IO); + hose->region_count = 3;
/* Set BAR0 to internal registers */ writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0)); @@ -442,6 +460,14 @@ static int mvebu_pcie_ofdata_to_platdata(struct udevice *dev) goto err; }
+ ret = mvebu_get_tgt_attr(dev_ofnode(dev->parent), pcie->devfn, + IORESOURCE_IO, + &pcie->io_target, &pcie->io_attr); + if (ret < 0) { + printf("%s: cannot get tgt/attr for IO window\n", pcie->name); + goto err; + } + /* Parse PCIe controller register base from DT */ ret = mvebu_pcie_port_parse_dt(dev_ofnode(dev), pcie); if (ret < 0)
On 03.01.21 23:06, Phil Sutter wrote:
Configure an IO region and window for PNP identical to how MEM region is set up. Linux does this only if the DT defines a pcie-io-aperture property for the SOC, but since all supported boards do this should not be needed.
Signed-off-by: Phil Sutter phil@nwl.cc
drivers/pci/pci_mvebu.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index c9afe7515014d..9528e7c40ecfe 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -73,6 +73,7 @@ struct mvebu_pcie { void __iomem *membase; struct resource mem; void __iomem *iobase;
- struct resource io; u32 port; u32 lane; int devfn;
@@ -81,6 +82,8 @@ struct mvebu_pcie { char name[16]; unsigned int mem_target; unsigned int mem_attr;
unsigned int io_target;
unsigned int io_attr; };
/*
@@ -90,6 +93,7 @@ struct mvebu_pcie { */ static void __iomem *mvebu_pcie_membase = (void __iomem *)MBUS_PCI_MEM_BASE; #define PCIE_MEM_SIZE (128 << 20) +static void __iomem *mvebu_pcie_iobase = (void __iomem *)MBUS_PCI_IO_BASE;
static inline bool mvebu_pcie_link_up(struct mvebu_pcie *pcie) { @@ -306,12 +310,24 @@ static int mvebu_pcie_probe(struct udevice *dev) (u32)pcie->mem.start, PCIE_MEM_SIZE); }
pcie->io.start = (u32)mvebu_pcie_iobase;
pcie->io.end = pcie->io.start + MBUS_PCI_IO_SIZE - 1;
mvebu_pcie_iobase += MBUS_PCI_IO_SIZE;
if (mvebu_mbus_add_window_by_id(pcie->io_target, pcie->io_attr,
(phys_addr_t)pcie->io.start,MBUS_PCI_IO_SIZE)) {printf("PCIe unable to add mbus window for IO at %08x+%08x\n",(u32)pcie->io.start, MBUS_PCI_IO_SIZE);}
/* Setup windows and configure host bridge */ mvebu_pcie_setup_wins(pcie);
/* Master + slave enable. */ reg = readl(pcie->base + PCIE_CMD_OFF); reg |= PCI_COMMAND_MEMORY;
reg |= PCI_COMMAND_IO; reg |= PCI_COMMAND_MASTER; reg |= BIT(10); /* disable interrupts */ writel(reg, pcie->base + PCIE_CMD_OFF);
@@ -323,7 +339,9 @@ static int mvebu_pcie_probe(struct udevice *dev) 0, 0, gd->ram_size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
- hose->region_count = 2;
pci_set_region(hose->regions + 2, pcie->io.start,
pcie->io.start, MBUS_PCI_IO_SIZE, PCI_REGION_IO);hose->region_count = 3;
/* Set BAR0 to internal registers */ writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
@@ -442,6 +460,14 @@ static int mvebu_pcie_ofdata_to_platdata(struct udevice *dev) goto err; }
- ret = mvebu_get_tgt_attr(dev_ofnode(dev->parent), pcie->devfn,
IORESOURCE_IO,&pcie->io_target, &pcie->io_attr);- if (ret < 0) {
printf("%s: cannot get tgt/attr for IO window\n", pcie->name);goto err;- }
- /* Parse PCIe controller register base from DT */ ret = mvebu_pcie_port_parse_dt(dev_ofnode(dev), pcie); if (ret < 0)
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
participants (3)
-
Phil Sutter -
Simon Glass -
Stefan Roese