| Message ID | 20170608141342.2018-3-lorenzo.pieralisi@arm.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi Lorenzo, Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week. I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed? If so, is there a repo I can pull them? Thanks, Ray On 6/8/2017 7:13 AM, Lorenzo Pieralisi wrote: > Current iproc driver host bridge controller driver requires struct > pci_bus to be created in order to carry out PCI link checks with standard > PCI config space accessors. > > This struct pci_bus dependency is fictitious and burdens the driver > with unneeded constraints (eg to use separate APIs to create and scan > the root bus). > > Add PCI raw config space accessors to PCIe iproc driver and remove the > fictitious struct pci_bus dependency. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Scott Branden <sbranden@broadcom.com> > Cc: Ray Jui <rjui@broadcom.com> > Cc: Jon Mason <jonmason@broadcom.com> > --- > drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++--------- > 1 file changed, 74 insertions(+), 20 deletions(-) > > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > index 0f39bd2..e48b8e2 100644 > --- a/drivers/pci/host/pcie-iproc.c > +++ b/drivers/pci/host/pcie-iproc.c > @@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus, > * Note access to the configuration registers are protected at the higher layer > * by 'pci_lock' in drivers/pci/access.c > */ > -static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, > +static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie, > + int busno, > unsigned int devfn, > int where) > { > - struct iproc_pcie *pcie = iproc_data(bus); > unsigned slot = PCI_SLOT(devfn); > unsigned fn = PCI_FUNC(devfn); > - unsigned busno = bus->number; > u32 val; > u16 offset; > > @@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, > return (pcie->base + offset); > } > > +static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus, > + unsigned int devfn, > + int where) > +{ > + return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn, > + where); > +} > + > +static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie, > + unsigned int devfn, int where, > + int size, u32 *val) > +{ > + void __iomem *addr; > + > + addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); > + if (!addr) { > + *val = ~0; > + return PCIBIOS_DEVICE_NOT_FOUND; > + } > + > + *val = readl(addr); > + > + if (size <= 2) > + *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1); > + > + return PCIBIOS_SUCCESSFUL; > +} > + > +static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie, > + unsigned int devfn, int where, > + int size, u32 val) > +{ > + void __iomem *addr; > + u32 mask, tmp; > + > + addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); > + if (!addr) > + return PCIBIOS_DEVICE_NOT_FOUND; > + > + if (size == 4) { > + writel(val, addr); > + return PCIBIOS_SUCCESSFUL; > + } > + > + mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8)); > + tmp = readl(addr) & mask; > + tmp |= val << ((where & 0x3) * 8); > + writel(tmp, addr); > + > + return PCIBIOS_SUCCESSFUL; > +} > + > static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn, > int where, int size, u32 *val) > { > @@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn, > } > > static struct pci_ops iproc_pcie_ops = { > - .map_bus = iproc_pcie_map_cfg_bus, > + .map_bus = iproc_pcie_bus_map_cfg_bus, > .read = iproc_pcie_config_read32, > .write = iproc_pcie_config_write32, > }; > @@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie) > msleep(100); > } > > -static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > +static int iproc_pcie_check_link(struct iproc_pcie *pcie) > { > struct device *dev = pcie->dev; > - u8 hdr_type; > - u32 link_ctrl, class, val; > - u16 pos = PCI_EXP_CAP, link_status; > + u32 hdr_type, link_ctrl, link_status, class, val; > + u16 pos = PCI_EXP_CAP; > bool link_is_active = false; > > /* > @@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > } > > /* make sure we are not in EP mode */ > - pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type); > + iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type); > if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) { > dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type); > return -EFAULT; > @@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c > #define PCI_CLASS_BRIDGE_MASK 0xffff00 > #define PCI_CLASS_BRIDGE_SHIFT 8 > - pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class); > + iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > + 4, &class); > class &= ~PCI_CLASS_BRIDGE_MASK; > class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT); > - pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class); > + iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > + 4, class); > > /* check link status to see if link is active */ > - pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status); > + iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA, > + 2, &link_status); > if (link_status & PCI_EXP_LNKSTA_NLW) > link_is_active = true; > > @@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > #define PCI_TARGET_LINK_SPEED_MASK 0xf > #define PCI_TARGET_LINK_SPEED_GEN2 0x2 > #define PCI_TARGET_LINK_SPEED_GEN1 0x1 > - pci_bus_read_config_dword(bus, 0, > - pos + PCI_EXP_LNKCTL2, > + iproc_pci_raw_config_read32(pcie, 0, > + pos + PCI_EXP_LNKCTL2, 4, > &link_ctrl); > if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) == > PCI_TARGET_LINK_SPEED_GEN2) { > link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK; > link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1; > - pci_bus_write_config_dword(bus, 0, > - pos + PCI_EXP_LNKCTL2, > - link_ctrl); > + iproc_pci_raw_config_write32(pcie, 0, > + pos + PCI_EXP_LNKCTL2, > + 4, link_ctrl); > msleep(100); > > - pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, > - &link_status); > + iproc_pci_raw_config_read32(pcie, 0, > + pos + PCI_EXP_LNKSTA, > + 2, &link_status); > if (link_status & PCI_EXP_LNKSTA_NLW) > link_is_active = true; > } > @@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) > } > pcie->root_bus = bus; > > - ret = iproc_pcie_check_link(pcie, bus); > + ret = iproc_pcie_check_link(pcie); > if (ret) { > dev_err(dev, "no PCIe EP device detected\n"); > goto err_rm_root_bus;
[dropped rock-chips maintainers, email bounces] On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: > Hi Lorenzo, > > Thanks, I'll try my best to find time to test this along 15/42 and > 33/42 patches. Hopefully I can get to that some time next week. > > I have not yet reviewed these patches in details. Do they have > dependency on other patches to the generic framework code you > changed? > > If so, is there a repo I can pull them? I added it in the cover letter but anyway here it is: git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 Thanks for testing it, please let me know. Thank you ! Lorenzo > > Thanks, > > Ray > > > On 6/8/2017 7:13 AM, Lorenzo Pieralisi wrote: > >Current iproc driver host bridge controller driver requires struct > >pci_bus to be created in order to carry out PCI link checks with standard > >PCI config space accessors. > > > >This struct pci_bus dependency is fictitious and burdens the driver > >with unneeded constraints (eg to use separate APIs to create and scan > >the root bus). > > > >Add PCI raw config space accessors to PCIe iproc driver and remove the > >fictitious struct pci_bus dependency. > > > >Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > >Cc: Scott Branden <sbranden@broadcom.com> > >Cc: Ray Jui <rjui@broadcom.com> > >Cc: Jon Mason <jonmason@broadcom.com> > >--- > > drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++--------- > > 1 file changed, 74 insertions(+), 20 deletions(-) > > > >diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > >index 0f39bd2..e48b8e2 100644 > >--- a/drivers/pci/host/pcie-iproc.c > >+++ b/drivers/pci/host/pcie-iproc.c > >@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus, > > * Note access to the configuration registers are protected at the higher layer > > * by 'pci_lock' in drivers/pci/access.c > > */ > >-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, > >+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie, > >+ int busno, > > unsigned int devfn, > > int where) > > { > >- struct iproc_pcie *pcie = iproc_data(bus); > > unsigned slot = PCI_SLOT(devfn); > > unsigned fn = PCI_FUNC(devfn); > >- unsigned busno = bus->number; > > u32 val; > > u16 offset; > >@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, > > return (pcie->base + offset); > > } > >+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus, > >+ unsigned int devfn, > >+ int where) > >+{ > >+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn, > >+ where); > >+} > >+ > >+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie, > >+ unsigned int devfn, int where, > >+ int size, u32 *val) > >+{ > >+ void __iomem *addr; > >+ > >+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); > >+ if (!addr) { > >+ *val = ~0; > >+ return PCIBIOS_DEVICE_NOT_FOUND; > >+ } > >+ > >+ *val = readl(addr); > >+ > >+ if (size <= 2) > >+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1); > >+ > >+ return PCIBIOS_SUCCESSFUL; > >+} > >+ > >+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie, > >+ unsigned int devfn, int where, > >+ int size, u32 val) > >+{ > >+ void __iomem *addr; > >+ u32 mask, tmp; > >+ > >+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); > >+ if (!addr) > >+ return PCIBIOS_DEVICE_NOT_FOUND; > >+ > >+ if (size == 4) { > >+ writel(val, addr); > >+ return PCIBIOS_SUCCESSFUL; > >+ } > >+ > >+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8)); > >+ tmp = readl(addr) & mask; > >+ tmp |= val << ((where & 0x3) * 8); > >+ writel(tmp, addr); > >+ > >+ return PCIBIOS_SUCCESSFUL; > >+} > >+ > > static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn, > > int where, int size, u32 *val) > > { > >@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn, > > } > > static struct pci_ops iproc_pcie_ops = { > >- .map_bus = iproc_pcie_map_cfg_bus, > >+ .map_bus = iproc_pcie_bus_map_cfg_bus, > > .read = iproc_pcie_config_read32, > > .write = iproc_pcie_config_write32, > > }; > >@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie) > > msleep(100); > > } > >-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > >+static int iproc_pcie_check_link(struct iproc_pcie *pcie) > > { > > struct device *dev = pcie->dev; > >- u8 hdr_type; > >- u32 link_ctrl, class, val; > >- u16 pos = PCI_EXP_CAP, link_status; > >+ u32 hdr_type, link_ctrl, link_status, class, val; > >+ u16 pos = PCI_EXP_CAP; > > bool link_is_active = false; > > /* > >@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > > } > > /* make sure we are not in EP mode */ > >- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type); > >+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type); > > if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) { > > dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type); > > return -EFAULT; > >@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > > #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c > > #define PCI_CLASS_BRIDGE_MASK 0xffff00 > > #define PCI_CLASS_BRIDGE_SHIFT 8 > >- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class); > >+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > >+ 4, &class); > > class &= ~PCI_CLASS_BRIDGE_MASK; > > class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT); > >- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class); > >+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > >+ 4, class); > > /* check link status to see if link is active */ > >- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status); > >+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA, > >+ 2, &link_status); > > if (link_status & PCI_EXP_LNKSTA_NLW) > > link_is_active = true; > >@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > > #define PCI_TARGET_LINK_SPEED_MASK 0xf > > #define PCI_TARGET_LINK_SPEED_GEN2 0x2 > > #define PCI_TARGET_LINK_SPEED_GEN1 0x1 > >- pci_bus_read_config_dword(bus, 0, > >- pos + PCI_EXP_LNKCTL2, > >+ iproc_pci_raw_config_read32(pcie, 0, > >+ pos + PCI_EXP_LNKCTL2, 4, > > &link_ctrl); > > if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) == > > PCI_TARGET_LINK_SPEED_GEN2) { > > link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK; > > link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1; > >- pci_bus_write_config_dword(bus, 0, > >- pos + PCI_EXP_LNKCTL2, > >- link_ctrl); > >+ iproc_pci_raw_config_write32(pcie, 0, > >+ pos + PCI_EXP_LNKCTL2, > >+ 4, link_ctrl); > > msleep(100); > >- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, > >- &link_status); > >+ iproc_pci_raw_config_read32(pcie, 0, > >+ pos + PCI_EXP_LNKSTA, > >+ 2, &link_status); > > if (link_status & PCI_EXP_LNKSTA_NLW) > > link_is_active = true; > > } > >@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) > > } > > pcie->root_bus = bus; > >- ret = iproc_pcie_check_link(pcie, bus); > >+ ret = iproc_pcie_check_link(pcie); > > if (ret) { > > dev_err(dev, "no PCIe EP device detected\n"); > > goto err_rm_root_bus; >
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > [dropped rock-chips maintainers, email bounces] > > On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: >> Hi Lorenzo, >> >> Thanks, I'll try my best to find time to test this along 15/42 and >> 33/42 patches. Hopefully I can get to that some time next week. >> >> I have not yet reviewed these patches in details. Do they have >> dependency on other patches to the generic framework code you >> changed? >> >> If so, is there a repo I can pull them? > > I added it in the cover letter but anyway here it is: > > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 Hi Lorenzo, I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay. Hi Bjorn, please help to review following changes, to avoid merge conflicts since Lorenzo's changes in iproc driver is touching basic config APIs. [PATCH v3 1/2] PCI: iproc: Retry request when CRS returned from EP [PATCH v3 2/2] PCI: iproc: add device shutdown for PCI RC Regards, Oza. > > Thanks for testing it, please let me know. > > Thank you ! > Lorenzo > >> >> Thanks, >> >> Ray >> >> >> On 6/8/2017 7:13 AM, Lorenzo Pieralisi wrote: >> >Current iproc driver host bridge controller driver requires struct >> >pci_bus to be created in order to carry out PCI link checks with standard >> >PCI config space accessors. >> > >> >This struct pci_bus dependency is fictitious and burdens the driver >> >with unneeded constraints (eg to use separate APIs to create and scan >> >the root bus). >> > >> >Add PCI raw config space accessors to PCIe iproc driver and remove the >> >fictitious struct pci_bus dependency. >> > >> >Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> >Cc: Scott Branden <sbranden@broadcom.com> >> >Cc: Ray Jui <rjui@broadcom.com> >> >Cc: Jon Mason <jonmason@broadcom.com> >> >--- >> > drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++--------- >> > 1 file changed, 74 insertions(+), 20 deletions(-) >> > >> >diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c >> >index 0f39bd2..e48b8e2 100644 >> >--- a/drivers/pci/host/pcie-iproc.c >> >+++ b/drivers/pci/host/pcie-iproc.c >> >@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus, >> > * Note access to the configuration registers are protected at the higher layer >> > * by 'pci_lock' in drivers/pci/access.c >> > */ >> >-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, >> >+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie, >> >+ int busno, >> > unsigned int devfn, >> > int where) >> > { >> >- struct iproc_pcie *pcie = iproc_data(bus); >> > unsigned slot = PCI_SLOT(devfn); >> > unsigned fn = PCI_FUNC(devfn); >> >- unsigned busno = bus->number; >> > u32 val; >> > u16 offset; >> >@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, >> > return (pcie->base + offset); >> > } >> >+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus, >> >+ unsigned int devfn, >> >+ int where) >> >+{ >> >+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn, >> >+ where); >> >+} >> >+ >> >+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie, >> >+ unsigned int devfn, int where, >> >+ int size, u32 *val) >> >+{ >> >+ void __iomem *addr; >> >+ >> >+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); >> >+ if (!addr) { >> >+ *val = ~0; >> >+ return PCIBIOS_DEVICE_NOT_FOUND; >> >+ } >> >+ >> >+ *val = readl(addr); >> >+ >> >+ if (size <= 2) >> >+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1); >> >+ >> >+ return PCIBIOS_SUCCESSFUL; >> >+} >> >+ >> >+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie, >> >+ unsigned int devfn, int where, >> >+ int size, u32 val) >> >+{ >> >+ void __iomem *addr; >> >+ u32 mask, tmp; >> >+ >> >+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); >> >+ if (!addr) >> >+ return PCIBIOS_DEVICE_NOT_FOUND; >> >+ >> >+ if (size == 4) { >> >+ writel(val, addr); >> >+ return PCIBIOS_SUCCESSFUL; >> >+ } >> >+ >> >+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8)); >> >+ tmp = readl(addr) & mask; >> >+ tmp |= val << ((where & 0x3) * 8); >> >+ writel(tmp, addr); >> >+ >> >+ return PCIBIOS_SUCCESSFUL; >> >+} >> >+ >> > static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn, >> > int where, int size, u32 *val) >> > { >> >@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn, >> > } >> > static struct pci_ops iproc_pcie_ops = { >> >- .map_bus = iproc_pcie_map_cfg_bus, >> >+ .map_bus = iproc_pcie_bus_map_cfg_bus, >> > .read = iproc_pcie_config_read32, >> > .write = iproc_pcie_config_write32, >> > }; >> >@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie) >> > msleep(100); >> > } >> >-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) >> >+static int iproc_pcie_check_link(struct iproc_pcie *pcie) >> > { >> > struct device *dev = pcie->dev; >> >- u8 hdr_type; >> >- u32 link_ctrl, class, val; >> >- u16 pos = PCI_EXP_CAP, link_status; >> >+ u32 hdr_type, link_ctrl, link_status, class, val; >> >+ u16 pos = PCI_EXP_CAP; >> > bool link_is_active = false; >> > /* >> >@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) >> > } >> > /* make sure we are not in EP mode */ >> >- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type); >> >+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type); >> > if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) { >> > dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type); >> > return -EFAULT; >> >@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) >> > #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c >> > #define PCI_CLASS_BRIDGE_MASK 0xffff00 >> > #define PCI_CLASS_BRIDGE_SHIFT 8 >> >- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class); >> >+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, >> >+ 4, &class); >> > class &= ~PCI_CLASS_BRIDGE_MASK; >> > class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT); >> >- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class); >> >+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, >> >+ 4, class); >> > /* check link status to see if link is active */ >> >- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status); >> >+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA, >> >+ 2, &link_status); >> > if (link_status & PCI_EXP_LNKSTA_NLW) >> > link_is_active = true; >> >@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) >> > #define PCI_TARGET_LINK_SPEED_MASK 0xf >> > #define PCI_TARGET_LINK_SPEED_GEN2 0x2 >> > #define PCI_TARGET_LINK_SPEED_GEN1 0x1 >> >- pci_bus_read_config_dword(bus, 0, >> >- pos + PCI_EXP_LNKCTL2, >> >+ iproc_pci_raw_config_read32(pcie, 0, >> >+ pos + PCI_EXP_LNKCTL2, 4, >> > &link_ctrl); >> > if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) == >> > PCI_TARGET_LINK_SPEED_GEN2) { >> > link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK; >> > link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1; >> >- pci_bus_write_config_dword(bus, 0, >> >- pos + PCI_EXP_LNKCTL2, >> >- link_ctrl); >> >+ iproc_pci_raw_config_write32(pcie, 0, >> >+ pos + PCI_EXP_LNKCTL2, >> >+ 4, link_ctrl); >> > msleep(100); >> >- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, >> >- &link_status); >> >+ iproc_pci_raw_config_read32(pcie, 0, >> >+ pos + PCI_EXP_LNKSTA, >> >+ 2, &link_status); >> > if (link_status & PCI_EXP_LNKSTA_NLW) >> > link_is_active = true; >> > } >> >@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) >> > } >> > pcie->root_bus = bus; >> >- ret = iproc_pcie_check_link(pcie, bus); >> >+ ret = iproc_pcie_check_link(pcie); >> > if (ret) { >> > dev_err(dev, "no PCIe EP device detected\n"); >> > goto err_rm_root_bus; >> > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: > On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: > > [dropped rock-chips maintainers, email bounces] > > > > On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: > >> Hi Lorenzo, > >> > >> Thanks, I'll try my best to find time to test this along 15/42 and > >> 33/42 patches. Hopefully I can get to that some time next week. > >> > >> I have not yet reviewed these patches in details. Do they have > >> dependency on other patches to the generic framework code you > >> changed? > >> > >> If so, is there a repo I can pull them? > > > > I added it in the cover letter but anyway here it is: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 > > Hi Lorenzo, > > I picked up your changes, and tested on iproc based SOC, > and ran basic fio data transfer.. and it looks okay. Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful. Thanks a lot, Lorenzo > Hi Bjorn, > > please help to review following changes, to avoid merge conflicts > since Lorenzo's changes in iproc driver is touching basic config APIs. > > [PATCH v3 1/2] PCI: iproc: Retry request when CRS returned from EP > [PATCH v3 2/2] PCI: iproc: add device shutdown for PCI RC > > Regards, > Oza. > > > > > > > Thanks for testing it, please let me know. > > > > Thank you ! > > Lorenzo > > > >> > >> Thanks, > >> > >> Ray > >> > >> > >> On 6/8/2017 7:13 AM, Lorenzo Pieralisi wrote: > >> >Current iproc driver host bridge controller driver requires struct > >> >pci_bus to be created in order to carry out PCI link checks with standard > >> >PCI config space accessors. > >> > > >> >This struct pci_bus dependency is fictitious and burdens the driver > >> >with unneeded constraints (eg to use separate APIs to create and scan > >> >the root bus). > >> > > >> >Add PCI raw config space accessors to PCIe iproc driver and remove the > >> >fictitious struct pci_bus dependency. > >> > > >> >Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > >> >Cc: Scott Branden <sbranden@broadcom.com> > >> >Cc: Ray Jui <rjui@broadcom.com> > >> >Cc: Jon Mason <jonmason@broadcom.com> > >> >--- > >> > drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++--------- > >> > 1 file changed, 74 insertions(+), 20 deletions(-) > >> > > >> >diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > >> >index 0f39bd2..e48b8e2 100644 > >> >--- a/drivers/pci/host/pcie-iproc.c > >> >+++ b/drivers/pci/host/pcie-iproc.c > >> >@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus, > >> > * Note access to the configuration registers are protected at the higher layer > >> > * by 'pci_lock' in drivers/pci/access.c > >> > */ > >> >-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, > >> >+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie, > >> >+ int busno, > >> > unsigned int devfn, > >> > int where) > >> > { > >> >- struct iproc_pcie *pcie = iproc_data(bus); > >> > unsigned slot = PCI_SLOT(devfn); > >> > unsigned fn = PCI_FUNC(devfn); > >> >- unsigned busno = bus->number; > >> > u32 val; > >> > u16 offset; > >> >@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, > >> > return (pcie->base + offset); > >> > } > >> >+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus, > >> >+ unsigned int devfn, > >> >+ int where) > >> >+{ > >> >+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn, > >> >+ where); > >> >+} > >> >+ > >> >+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie, > >> >+ unsigned int devfn, int where, > >> >+ int size, u32 *val) > >> >+{ > >> >+ void __iomem *addr; > >> >+ > >> >+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); > >> >+ if (!addr) { > >> >+ *val = ~0; > >> >+ return PCIBIOS_DEVICE_NOT_FOUND; > >> >+ } > >> >+ > >> >+ *val = readl(addr); > >> >+ > >> >+ if (size <= 2) > >> >+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1); > >> >+ > >> >+ return PCIBIOS_SUCCESSFUL; > >> >+} > >> >+ > >> >+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie, > >> >+ unsigned int devfn, int where, > >> >+ int size, u32 val) > >> >+{ > >> >+ void __iomem *addr; > >> >+ u32 mask, tmp; > >> >+ > >> >+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); > >> >+ if (!addr) > >> >+ return PCIBIOS_DEVICE_NOT_FOUND; > >> >+ > >> >+ if (size == 4) { > >> >+ writel(val, addr); > >> >+ return PCIBIOS_SUCCESSFUL; > >> >+ } > >> >+ > >> >+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8)); > >> >+ tmp = readl(addr) & mask; > >> >+ tmp |= val << ((where & 0x3) * 8); > >> >+ writel(tmp, addr); > >> >+ > >> >+ return PCIBIOS_SUCCESSFUL; > >> >+} > >> >+ > >> > static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn, > >> > int where, int size, u32 *val) > >> > { > >> >@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn, > >> > } > >> > static struct pci_ops iproc_pcie_ops = { > >> >- .map_bus = iproc_pcie_map_cfg_bus, > >> >+ .map_bus = iproc_pcie_bus_map_cfg_bus, > >> > .read = iproc_pcie_config_read32, > >> > .write = iproc_pcie_config_write32, > >> > }; > >> >@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie) > >> > msleep(100); > >> > } > >> >-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > >> >+static int iproc_pcie_check_link(struct iproc_pcie *pcie) > >> > { > >> > struct device *dev = pcie->dev; > >> >- u8 hdr_type; > >> >- u32 link_ctrl, class, val; > >> >- u16 pos = PCI_EXP_CAP, link_status; > >> >+ u32 hdr_type, link_ctrl, link_status, class, val; > >> >+ u16 pos = PCI_EXP_CAP; > >> > bool link_is_active = false; > >> > /* > >> >@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > >> > } > >> > /* make sure we are not in EP mode */ > >> >- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type); > >> >+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type); > >> > if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) { > >> > dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type); > >> > return -EFAULT; > >> >@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > >> > #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c > >> > #define PCI_CLASS_BRIDGE_MASK 0xffff00 > >> > #define PCI_CLASS_BRIDGE_SHIFT 8 > >> >- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class); > >> >+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > >> >+ 4, &class); > >> > class &= ~PCI_CLASS_BRIDGE_MASK; > >> > class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT); > >> >- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class); > >> >+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, > >> >+ 4, class); > >> > /* check link status to see if link is active */ > >> >- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status); > >> >+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA, > >> >+ 2, &link_status); > >> > if (link_status & PCI_EXP_LNKSTA_NLW) > >> > link_is_active = true; > >> >@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) > >> > #define PCI_TARGET_LINK_SPEED_MASK 0xf > >> > #define PCI_TARGET_LINK_SPEED_GEN2 0x2 > >> > #define PCI_TARGET_LINK_SPEED_GEN1 0x1 > >> >- pci_bus_read_config_dword(bus, 0, > >> >- pos + PCI_EXP_LNKCTL2, > >> >+ iproc_pci_raw_config_read32(pcie, 0, > >> >+ pos + PCI_EXP_LNKCTL2, 4, > >> > &link_ctrl); > >> > if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) == > >> > PCI_TARGET_LINK_SPEED_GEN2) { > >> > link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK; > >> > link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1; > >> >- pci_bus_write_config_dword(bus, 0, > >> >- pos + PCI_EXP_LNKCTL2, > >> >- link_ctrl); > >> >+ iproc_pci_raw_config_write32(pcie, 0, > >> >+ pos + PCI_EXP_LNKCTL2, > >> >+ 4, link_ctrl); > >> > msleep(100); > >> >- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, > >> >- &link_status); > >> >+ iproc_pci_raw_config_read32(pcie, 0, > >> >+ pos + PCI_EXP_LNKSTA, > >> >+ 2, &link_status); > >> > if (link_status & PCI_EXP_LNKSTA_NLW) > >> > link_is_active = true; > >> > } > >> >@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) > >> > } > >> > pcie->root_bus = bus; > >> >- ret = iproc_pcie_check_link(pcie, bus); > >> >+ ret = iproc_pcie_check_link(pcie); > >> > if (ret) { > >> > dev_err(dev, "no PCIe EP device detected\n"); > >> > goto err_rm_root_bus; > >> > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: >> On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi >> <lorenzo.pieralisi@arm.com> wrote: >> > [dropped rock-chips maintainers, email bounces] >> > >> > On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: >> >> Hi Lorenzo, >> >> >> >> Thanks, I'll try my best to find time to test this along 15/42 and >> >> 33/42 patches. Hopefully I can get to that some time next week. >> >> >> >> I have not yet reviewed these patches in details. Do they have >> >> dependency on other patches to the generic framework code you >> >> changed? >> >> >> >> If so, is there a repo I can pull them? >> > >> > I added it in the cover letter but anyway here it is: >> > >> > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 >> >> Hi Lorenzo, >> >> I picked up your changes, and tested on iproc based SOC, >> and ran basic fio data transfer.. and it looks okay. > > Thank you. If you could also check it is all OK from a legacy > IRQ allocation (ie same as before applying series) I'd be grateful. > I disabled msi and it should default to legacy IRQ, looks like there is a problem, if I missed any change ! before applying series: cat /proc/interrupts 377: 168 0 0 0 0 0 0 0 dummy 1 Edge nvme0q0, nvme0q1 after applying series: root@bcm958742k:~# dmesg | grep nvme [ 3.855466] nvme 0000:01:00.0: assign irq: got 0 [ 3.855469] nvme 0000:01:00.0: assigning IRQ 00 [ 3.855515] nvme nvme0: pci function 0000:01:00.0 [ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002) [ 4.276787] nvme 0000:01:00.0: enabling bus mastering [ 4.276817] nvme nvme0: Removing after probe failure status: -22 here is my git status of your series, let me know if I am missing any change with respect to legacy IRQ ? modified: arch/arm/include/asm/mach/pci.h modified: arch/arm/kernel/bios32.c modified: arch/arm/mach-dove/pcie.c modified: arch/arm/mach-iop13xx/pci.c modified: arch/arm/mach-iop13xx/pci.h modified: arch/arm/mach-mv78xx0/pcie.c modified: arch/arm/mach-orion5x/common.h modified: arch/arm/mach-orion5x/pci.c modified: arch/arm64/kernel/pci.c modified: drivers/of/of_pci_irq.c modified: drivers/pci/Makefile modified: drivers/pci/host/pci-aardvark.c modified: drivers/pci/host/pci-ftpci100.c modified: drivers/pci/host/pci-host-common.c modified: drivers/pci/host/pci-tegra.c modified: drivers/pci/host/pci-versatile.c modified: drivers/pci/host/pci-xgene.c modified: drivers/pci/host/pcie-altera.c modified: drivers/pci/host/pcie-iproc-bcma.c modified: drivers/pci/host/pcie-iproc-platform.c modified: drivers/pci/host/pcie-iproc.c modified: drivers/pci/host/pcie-iproc.h modified: drivers/pci/host/pcie-rcar.c modified: drivers/pci/host/pcie-rockchip.c modified: drivers/pci/host/pcie-xilinx-nwl.c modified: drivers/pci/host/pcie-xilinx.c modified: drivers/pci/pci-driver.c modified: drivers/pci/probe.c modified: drivers/pci/setup-irq.c modified: include/linux/pci.h > Thanks a lot, > Lorenzo > >> Hi Bjorn, >> >> please help to review following changes, to avoid merge conflicts >> since Lorenzo's changes in iproc driver is touching basic config APIs. >> >> [PATCH v3 1/2] PCI: iproc: Retry request when CRS returned from EP >> [PATCH v3 2/2] PCI: iproc: add device shutdown for PCI RC >> >> Regards, >> Oza. >> >> >> >> > >> > Thanks for testing it, please let me know. >> > >> > Thank you ! >> > Lorenzo >> > >> >> >> >> Thanks, >> >> >> >> Ray >> >> >> >> >> >> On 6/8/2017 7:13 AM, Lorenzo Pieralisi wrote: >> >> >Current iproc driver host bridge controller driver requires struct >> >> >pci_bus to be created in order to carry out PCI link checks with standard >> >> >PCI config space accessors. >> >> > >> >> >This struct pci_bus dependency is fictitious and burdens the driver >> >> >with unneeded constraints (eg to use separate APIs to create and scan >> >> >the root bus). >> >> > >> >> >Add PCI raw config space accessors to PCIe iproc driver and remove the >> >> >fictitious struct pci_bus dependency. >> >> > >> >> >Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> >> >Cc: Scott Branden <sbranden@broadcom.com> >> >> >Cc: Ray Jui <rjui@broadcom.com> >> >> >Cc: Jon Mason <jonmason@broadcom.com> >> >> >--- >> >> > drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++--------- >> >> > 1 file changed, 74 insertions(+), 20 deletions(-) >> >> > >> >> >diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c >> >> >index 0f39bd2..e48b8e2 100644 >> >> >--- a/drivers/pci/host/pcie-iproc.c >> >> >+++ b/drivers/pci/host/pcie-iproc.c >> >> >@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus, >> >> > * Note access to the configuration registers are protected at the higher layer >> >> > * by 'pci_lock' in drivers/pci/access.c >> >> > */ >> >> >-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, >> >> >+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie, >> >> >+ int busno, >> >> > unsigned int devfn, >> >> > int where) >> >> > { >> >> >- struct iproc_pcie *pcie = iproc_data(bus); >> >> > unsigned slot = PCI_SLOT(devfn); >> >> > unsigned fn = PCI_FUNC(devfn); >> >> >- unsigned busno = bus->number; >> >> > u32 val; >> >> > u16 offset; >> >> >@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, >> >> > return (pcie->base + offset); >> >> > } >> >> >+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus, >> >> >+ unsigned int devfn, >> >> >+ int where) >> >> >+{ >> >> >+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn, >> >> >+ where); >> >> >+} >> >> >+ >> >> >+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie, >> >> >+ unsigned int devfn, int where, >> >> >+ int size, u32 *val) >> >> >+{ >> >> >+ void __iomem *addr; >> >> >+ >> >> >+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); >> >> >+ if (!addr) { >> >> >+ *val = ~0; >> >> >+ return PCIBIOS_DEVICE_NOT_FOUND; >> >> >+ } >> >> >+ >> >> >+ *val = readl(addr); >> >> >+ >> >> >+ if (size <= 2) >> >> >+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1); >> >> >+ >> >> >+ return PCIBIOS_SUCCESSFUL; >> >> >+} >> >> >+ >> >> >+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie, >> >> >+ unsigned int devfn, int where, >> >> >+ int size, u32 val) >> >> >+{ >> >> >+ void __iomem *addr; >> >> >+ u32 mask, tmp; >> >> >+ >> >> >+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); >> >> >+ if (!addr) >> >> >+ return PCIBIOS_DEVICE_NOT_FOUND; >> >> >+ >> >> >+ if (size == 4) { >> >> >+ writel(val, addr); >> >> >+ return PCIBIOS_SUCCESSFUL; >> >> >+ } >> >> >+ >> >> >+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8)); >> >> >+ tmp = readl(addr) & mask; >> >> >+ tmp |= val << ((where & 0x3) * 8); >> >> >+ writel(tmp, addr); >> >> >+ >> >> >+ return PCIBIOS_SUCCESSFUL; >> >> >+} >> >> >+ >> >> > static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn, >> >> > int where, int size, u32 *val) >> >> > { >> >> >@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn, >> >> > } >> >> > static struct pci_ops iproc_pcie_ops = { >> >> >- .map_bus = iproc_pcie_map_cfg_bus, >> >> >+ .map_bus = iproc_pcie_bus_map_cfg_bus, >> >> > .read = iproc_pcie_config_read32, >> >> > .write = iproc_pcie_config_write32, >> >> > }; >> >> >@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie) >> >> > msleep(100); >> >> > } >> >> >-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) >> >> >+static int iproc_pcie_check_link(struct iproc_pcie *pcie) >> >> > { >> >> > struct device *dev = pcie->dev; >> >> >- u8 hdr_type; >> >> >- u32 link_ctrl, class, val; >> >> >- u16 pos = PCI_EXP_CAP, link_status; >> >> >+ u32 hdr_type, link_ctrl, link_status, class, val; >> >> >+ u16 pos = PCI_EXP_CAP; >> >> > bool link_is_active = false; >> >> > /* >> >> >@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) >> >> > } >> >> > /* make sure we are not in EP mode */ >> >> >- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type); >> >> >+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type); >> >> > if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) { >> >> > dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type); >> >> > return -EFAULT; >> >> >@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) >> >> > #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c >> >> > #define PCI_CLASS_BRIDGE_MASK 0xffff00 >> >> > #define PCI_CLASS_BRIDGE_SHIFT 8 >> >> >- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class); >> >> >+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, >> >> >+ 4, &class); >> >> > class &= ~PCI_CLASS_BRIDGE_MASK; >> >> > class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT); >> >> >- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class); >> >> >+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, >> >> >+ 4, class); >> >> > /* check link status to see if link is active */ >> >> >- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status); >> >> >+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA, >> >> >+ 2, &link_status); >> >> > if (link_status & PCI_EXP_LNKSTA_NLW) >> >> > link_is_active = true; >> >> >@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) >> >> > #define PCI_TARGET_LINK_SPEED_MASK 0xf >> >> > #define PCI_TARGET_LINK_SPEED_GEN2 0x2 >> >> > #define PCI_TARGET_LINK_SPEED_GEN1 0x1 >> >> >- pci_bus_read_config_dword(bus, 0, >> >> >- pos + PCI_EXP_LNKCTL2, >> >> >+ iproc_pci_raw_config_read32(pcie, 0, >> >> >+ pos + PCI_EXP_LNKCTL2, 4, >> >> > &link_ctrl); >> >> > if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) == >> >> > PCI_TARGET_LINK_SPEED_GEN2) { >> >> > link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK; >> >> > link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1; >> >> >- pci_bus_write_config_dword(bus, 0, >> >> >- pos + PCI_EXP_LNKCTL2, >> >> >- link_ctrl); >> >> >+ iproc_pci_raw_config_write32(pcie, 0, >> >> >+ pos + PCI_EXP_LNKCTL2, >> >> >+ 4, link_ctrl); >> >> > msleep(100); >> >> >- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, >> >> >- &link_status); >> >> >+ iproc_pci_raw_config_read32(pcie, 0, >> >> >+ pos + PCI_EXP_LNKSTA, >> >> >+ 2, &link_status); >> >> > if (link_status & PCI_EXP_LNKSTA_NLW) >> >> > link_is_active = true; >> >> > } >> >> >@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) >> >> > } >> >> > pcie->root_bus = bus; >> >> >- ret = iproc_pcie_check_link(pcie, bus); >> >> >+ ret = iproc_pcie_check_link(pcie); >> >> > if (ret) { >> >> > dev_err(dev, "no PCIe EP device detected\n"); >> >> > goto err_rm_root_bus; >> >> >> > >> > _______________________________________________ >> > linux-arm-kernel mailing list >> > linux-arm-kernel@lists.infradead.org >> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 6/12/17 10:40 AM, Oza Oza wrote: > On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: >> On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: >>> On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi >>> <lorenzo.pieralisi@arm.com> wrote: >>>> [dropped rock-chips maintainers, email bounces] >>>> >>>> On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: >>>>> Hi Lorenzo, >>>>> >>>>> Thanks, I'll try my best to find time to test this along 15/42 and >>>>> 33/42 patches. Hopefully I can get to that some time next week. >>>>> >>>>> I have not yet reviewed these patches in details. Do they have >>>>> dependency on other patches to the generic framework code you >>>>> changed? >>>>> >>>>> If so, is there a repo I can pull them? >>>> >>>> I added it in the cover letter but anyway here it is: >>>> >>>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 >>> >>> Hi Lorenzo, >>> >>> I picked up your changes, and tested on iproc based SOC, >>> and ran basic fio data transfer.. and it looks okay. >> >> Thank you. If you could also check it is all OK from a legacy >> IRQ allocation (ie same as before applying series) I'd be grateful. >> > > I disabled msi and it should default to legacy IRQ, looks like there > is a problem, if I missed any change ! > > before applying series: cat /proc/interrupts > 377: 168 0 0 0 0 0 > 0 0 dummy 1 Edge nvme0q0, nvme0q1 > > after applying series: > root@bcm958742k:~# dmesg | grep nvme > [ 3.855466] nvme 0000:01:00.0: assign irq: got 0 > [ 3.855469] nvme 0000:01:00.0: assigning IRQ 00 > [ 3.855515] nvme nvme0: pci function 0000:01:00.0 > [ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002) > [ 4.276787] nvme 0000:01:00.0: enabling bus mastering > [ 4.276817] nvme nvme0: Removing after probe failure status: -22 > > here is my git status of your series, let me know if I am missing any > change with respect to legacy IRQ ? > > modified: arch/arm/include/asm/mach/pci.h > modified: arch/arm/kernel/bios32.c > modified: arch/arm/mach-dove/pcie.c > modified: arch/arm/mach-iop13xx/pci.c > modified: arch/arm/mach-iop13xx/pci.h > modified: arch/arm/mach-mv78xx0/pcie.c > modified: arch/arm/mach-orion5x/common.h > modified: arch/arm/mach-orion5x/pci.c > modified: arch/arm64/kernel/pci.c > modified: drivers/of/of_pci_irq.c > modified: drivers/pci/Makefile > modified: drivers/pci/host/pci-aardvark.c > modified: drivers/pci/host/pci-ftpci100.c > modified: drivers/pci/host/pci-host-common.c > modified: drivers/pci/host/pci-tegra.c > modified: drivers/pci/host/pci-versatile.c > modified: drivers/pci/host/pci-xgene.c > modified: drivers/pci/host/pcie-altera.c > modified: drivers/pci/host/pcie-iproc-bcma.c > modified: drivers/pci/host/pcie-iproc-platform.c > modified: drivers/pci/host/pcie-iproc.c > modified: drivers/pci/host/pcie-iproc.h > modified: drivers/pci/host/pcie-rcar.c > modified: drivers/pci/host/pcie-rockchip.c > modified: drivers/pci/host/pcie-xilinx-nwl.c > modified: drivers/pci/host/pcie-xilinx.c > modified: drivers/pci/pci-driver.c > modified: drivers/pci/probe.c > modified: drivers/pci/setup-irq.c > modified: include/linux/pci.h > Did you test with or without my change in bcm/master to emulate legacy interrupt through dummy IRQ domain? Thanks, Ray
On Tue, Jun 13, 2017 at 12:22 AM, Ray Jui <ray.jui@broadcom.com> wrote: > > > On 6/12/17 10:40 AM, Oza Oza wrote: >> On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi >> <lorenzo.pieralisi@arm.com> wrote: >>> On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: >>>> On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi >>>> <lorenzo.pieralisi@arm.com> wrote: >>>>> [dropped rock-chips maintainers, email bounces] >>>>> >>>>> On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: >>>>>> Hi Lorenzo, >>>>>> >>>>>> Thanks, I'll try my best to find time to test this along 15/42 and >>>>>> 33/42 patches. Hopefully I can get to that some time next week. >>>>>> >>>>>> I have not yet reviewed these patches in details. Do they have >>>>>> dependency on other patches to the generic framework code you >>>>>> changed? >>>>>> >>>>>> If so, is there a repo I can pull them? >>>>> >>>>> I added it in the cover letter but anyway here it is: >>>>> >>>>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 >>>> >>>> Hi Lorenzo, >>>> >>>> I picked up your changes, and tested on iproc based SOC, >>>> and ran basic fio data transfer.. and it looks okay. >>> >>> Thank you. If you could also check it is all OK from a legacy >>> IRQ allocation (ie same as before applying series) I'd be grateful. >>> >> >> I disabled msi and it should default to legacy IRQ, looks like there >> is a problem, if I missed any change ! >> >> before applying series: cat /proc/interrupts >> 377: 168 0 0 0 0 0 >> 0 0 dummy 1 Edge nvme0q0, nvme0q1 >> >> after applying series: >> root@bcm958742k:~# dmesg | grep nvme >> [ 3.855466] nvme 0000:01:00.0: assign irq: got 0 >> [ 3.855469] nvme 0000:01:00.0: assigning IRQ 00 >> [ 3.855515] nvme nvme0: pci function 0000:01:00.0 >> [ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002) >> [ 4.276787] nvme 0000:01:00.0: enabling bus mastering >> [ 4.276817] nvme nvme0: Removing after probe failure status: -22 >> >> here is my git status of your series, let me know if I am missing any >> change with respect to legacy IRQ ? >> >> modified: arch/arm/include/asm/mach/pci.h >> modified: arch/arm/kernel/bios32.c >> modified: arch/arm/mach-dove/pcie.c >> modified: arch/arm/mach-iop13xx/pci.c >> modified: arch/arm/mach-iop13xx/pci.h >> modified: arch/arm/mach-mv78xx0/pcie.c >> modified: arch/arm/mach-orion5x/common.h >> modified: arch/arm/mach-orion5x/pci.c >> modified: arch/arm64/kernel/pci.c >> modified: drivers/of/of_pci_irq.c >> modified: drivers/pci/Makefile >> modified: drivers/pci/host/pci-aardvark.c >> modified: drivers/pci/host/pci-ftpci100.c >> modified: drivers/pci/host/pci-host-common.c >> modified: drivers/pci/host/pci-tegra.c >> modified: drivers/pci/host/pci-versatile.c >> modified: drivers/pci/host/pci-xgene.c >> modified: drivers/pci/host/pcie-altera.c >> modified: drivers/pci/host/pcie-iproc-bcma.c >> modified: drivers/pci/host/pcie-iproc-platform.c >> modified: drivers/pci/host/pcie-iproc.c >> modified: drivers/pci/host/pcie-iproc.h >> modified: drivers/pci/host/pcie-rcar.c >> modified: drivers/pci/host/pcie-rockchip.c >> modified: drivers/pci/host/pcie-xilinx-nwl.c >> modified: drivers/pci/host/pcie-xilinx.c >> modified: drivers/pci/pci-driver.c >> modified: drivers/pci/probe.c >> modified: drivers/pci/setup-irq.c >> modified: include/linux/pci.h >> > > Did you test with or without my change in bcm/master to emulate legacy > interrupt through dummy IRQ domain? > > Thanks, > > Ray > > Hi Ray, yes irqdomain was missing, and with that legacy IRQ are fine now. please mainline those changes. Hi Lorenzo, Legacy IRQ is working fine as well wiht your patches. cat /proc/interrupts 122: 160 0 0 0 0 0 0 0 dummy 1 Edge nvme0q0, nvme0q1 NVMe data transfer is also fine. Regards, Oza.
Hi Oza/Lorenzo, On 6/13/17 1:22 AM, Oza Oza wrote: > On Tue, Jun 13, 2017 at 12:22 AM, Ray Jui <ray.jui@broadcom.com> wrote: >> >> >> On 6/12/17 10:40 AM, Oza Oza wrote: >>> On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi >>> <lorenzo.pieralisi@arm.com> wrote: >>>> On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: >>>>> On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi >>>>> <lorenzo.pieralisi@arm.com> wrote: >>>>>> [dropped rock-chips maintainers, email bounces] >>>>>> >>>>>> On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: >>>>>>> Hi Lorenzo, >>>>>>> >>>>>>> Thanks, I'll try my best to find time to test this along 15/42 and >>>>>>> 33/42 patches. Hopefully I can get to that some time next week. >>>>>>> >>>>>>> I have not yet reviewed these patches in details. Do they have >>>>>>> dependency on other patches to the generic framework code you >>>>>>> changed? >>>>>>> >>>>>>> If so, is there a repo I can pull them? >>>>>> >>>>>> I added it in the cover letter but anyway here it is: >>>>>> >>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 >>>>> >>>>> Hi Lorenzo, >>>>> >>>>> I picked up your changes, and tested on iproc based SOC, >>>>> and ran basic fio data transfer.. and it looks okay. >>>> >>>> Thank you. If you could also check it is all OK from a legacy >>>> IRQ allocation (ie same as before applying series) I'd be grateful. >>>> >>> >>> I disabled msi and it should default to legacy IRQ, looks like there >>> is a problem, if I missed any change ! >>> >>> before applying series: cat /proc/interrupts >>> 377: 168 0 0 0 0 0 >>> 0 0 dummy 1 Edge nvme0q0, nvme0q1 >>> >>> after applying series: >>> root@bcm958742k:~# dmesg | grep nvme >>> [ 3.855466] nvme 0000:01:00.0: assign irq: got 0 >>> [ 3.855469] nvme 0000:01:00.0: assigning IRQ 00 >>> [ 3.855515] nvme nvme0: pci function 0000:01:00.0 >>> [ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002) >>> [ 4.276787] nvme 0000:01:00.0: enabling bus mastering >>> [ 4.276817] nvme nvme0: Removing after probe failure status: -22 >>> >>> here is my git status of your series, let me know if I am missing any >>> change with respect to legacy IRQ ? >>> >>> modified: arch/arm/include/asm/mach/pci.h >>> modified: arch/arm/kernel/bios32.c >>> modified: arch/arm/mach-dove/pcie.c >>> modified: arch/arm/mach-iop13xx/pci.c >>> modified: arch/arm/mach-iop13xx/pci.h >>> modified: arch/arm/mach-mv78xx0/pcie.c >>> modified: arch/arm/mach-orion5x/common.h >>> modified: arch/arm/mach-orion5x/pci.c >>> modified: arch/arm64/kernel/pci.c >>> modified: drivers/of/of_pci_irq.c >>> modified: drivers/pci/Makefile >>> modified: drivers/pci/host/pci-aardvark.c >>> modified: drivers/pci/host/pci-ftpci100.c >>> modified: drivers/pci/host/pci-host-common.c >>> modified: drivers/pci/host/pci-tegra.c >>> modified: drivers/pci/host/pci-versatile.c >>> modified: drivers/pci/host/pci-xgene.c >>> modified: drivers/pci/host/pcie-altera.c >>> modified: drivers/pci/host/pcie-iproc-bcma.c >>> modified: drivers/pci/host/pcie-iproc-platform.c >>> modified: drivers/pci/host/pcie-iproc.c >>> modified: drivers/pci/host/pcie-iproc.h >>> modified: drivers/pci/host/pcie-rcar.c >>> modified: drivers/pci/host/pcie-rockchip.c >>> modified: drivers/pci/host/pcie-xilinx-nwl.c >>> modified: drivers/pci/host/pcie-xilinx.c >>> modified: drivers/pci/pci-driver.c >>> modified: drivers/pci/probe.c >>> modified: drivers/pci/setup-irq.c >>> modified: include/linux/pci.h >>> >> >> Did you test with or without my change in bcm/master to emulate legacy >> interrupt through dummy IRQ domain? >> >> Thanks, >> >> Ray >> >> > > Hi Ray, > yes irqdomain was missing, and with that legacy IRQ are fine now. > please mainline those changes. > > Hi Lorenzo, > > Legacy IRQ is working fine as well wiht your patches. > > cat /proc/interrupts > 122: 160 0 0 0 0 0 > 0 0 dummy 1 Edge nvme0q0, nvme0q1 > > NVMe data transfer is also fine. > > Regards, > Oza. > Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been sanity tested for link detection and legacy interrupt support. No regression is seen. Thanks, Oza. I'll send out the INTx irqdomain support patch when I have time. Ray
On Tue, Jun 13, 2017 at 10:18:14AM -0700, Ray Jui wrote: > Hi Oza/Lorenzo, > > On 6/13/17 1:22 AM, Oza Oza wrote: > > On Tue, Jun 13, 2017 at 12:22 AM, Ray Jui <ray.jui@broadcom.com> wrote: > >> > >> > >> On 6/12/17 10:40 AM, Oza Oza wrote: > >>> On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi > >>> <lorenzo.pieralisi@arm.com> wrote: > >>>> On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: > >>>>> On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi > >>>>> <lorenzo.pieralisi@arm.com> wrote: > >>>>>> [dropped rock-chips maintainers, email bounces] > >>>>>> > >>>>>> On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: > >>>>>>> Hi Lorenzo, > >>>>>>> > >>>>>>> Thanks, I'll try my best to find time to test this along 15/42 and > >>>>>>> 33/42 patches. Hopefully I can get to that some time next week. > >>>>>>> > >>>>>>> I have not yet reviewed these patches in details. Do they have > >>>>>>> dependency on other patches to the generic framework code you > >>>>>>> changed? > >>>>>>> > >>>>>>> If so, is there a repo I can pull them? > >>>>>> > >>>>>> I added it in the cover letter but anyway here it is: > >>>>>> > >>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 > >>>>> > >>>>> Hi Lorenzo, > >>>>> > >>>>> I picked up your changes, and tested on iproc based SOC, > >>>>> and ran basic fio data transfer.. and it looks okay. > >>>> > >>>> Thank you. If you could also check it is all OK from a legacy > >>>> IRQ allocation (ie same as before applying series) I'd be grateful. > >>>> > >>> > >>> I disabled msi and it should default to legacy IRQ, looks like there > >>> is a problem, if I missed any change ! > >>> > >>> before applying series: cat /proc/interrupts > >>> 377: 168 0 0 0 0 0 > >>> 0 0 dummy 1 Edge nvme0q0, nvme0q1 > >>> > >>> after applying series: > >>> root@bcm958742k:~# dmesg | grep nvme > >>> [ 3.855466] nvme 0000:01:00.0: assign irq: got 0 > >>> [ 3.855469] nvme 0000:01:00.0: assigning IRQ 00 > >>> [ 3.855515] nvme nvme0: pci function 0000:01:00.0 > >>> [ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002) > >>> [ 4.276787] nvme 0000:01:00.0: enabling bus mastering > >>> [ 4.276817] nvme nvme0: Removing after probe failure status: -22 > >>> > >>> here is my git status of your series, let me know if I am missing any > >>> change with respect to legacy IRQ ? > >>> > >>> modified: arch/arm/include/asm/mach/pci.h > >>> modified: arch/arm/kernel/bios32.c > >>> modified: arch/arm/mach-dove/pcie.c > >>> modified: arch/arm/mach-iop13xx/pci.c > >>> modified: arch/arm/mach-iop13xx/pci.h > >>> modified: arch/arm/mach-mv78xx0/pcie.c > >>> modified: arch/arm/mach-orion5x/common.h > >>> modified: arch/arm/mach-orion5x/pci.c > >>> modified: arch/arm64/kernel/pci.c > >>> modified: drivers/of/of_pci_irq.c > >>> modified: drivers/pci/Makefile > >>> modified: drivers/pci/host/pci-aardvark.c > >>> modified: drivers/pci/host/pci-ftpci100.c > >>> modified: drivers/pci/host/pci-host-common.c > >>> modified: drivers/pci/host/pci-tegra.c > >>> modified: drivers/pci/host/pci-versatile.c > >>> modified: drivers/pci/host/pci-xgene.c > >>> modified: drivers/pci/host/pcie-altera.c > >>> modified: drivers/pci/host/pcie-iproc-bcma.c > >>> modified: drivers/pci/host/pcie-iproc-platform.c > >>> modified: drivers/pci/host/pcie-iproc.c > >>> modified: drivers/pci/host/pcie-iproc.h > >>> modified: drivers/pci/host/pcie-rcar.c > >>> modified: drivers/pci/host/pcie-rockchip.c > >>> modified: drivers/pci/host/pcie-xilinx-nwl.c > >>> modified: drivers/pci/host/pcie-xilinx.c > >>> modified: drivers/pci/pci-driver.c > >>> modified: drivers/pci/probe.c > >>> modified: drivers/pci/setup-irq.c > >>> modified: include/linux/pci.h > >>> > >> > >> Did you test with or without my change in bcm/master to emulate legacy > >> interrupt through dummy IRQ domain? > >> > >> Thanks, > >> > >> Ray > >> > >> > > > > Hi Ray, > > yes irqdomain was missing, and with that legacy IRQ are fine now. > > please mainline those changes. > > > > Hi Lorenzo, > > > > Legacy IRQ is working fine as well wiht your patches. > > > > cat /proc/interrupts > > 122: 160 0 0 0 0 0 > > 0 0 dummy 1 Edge nvme0q0, nvme0q1 > > > > NVMe data transfer is also fine. > > > > Regards, > > Oza. > > > > Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been > sanity tested for link detection and legacy interrupt support. No > regression is seen. Thanks, Oza. Yes, that's how I take it too, I did not understand what was triggering the first regression reported - my series should work on top of mainline if the current code works and should not depend on any other patch being merged - please make sure that's the case to prevent unwanted regressions in case other patches do not make it into the mainline. Thank you very much for your help in testing it. Lorenzo > > I'll send out the INTx irqdomain support patch when I have time. > > Ray > > > >
On Wed, Jun 14, 2017 at 7:09 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Tue, Jun 13, 2017 at 10:18:14AM -0700, Ray Jui wrote: >> Hi Oza/Lorenzo, >> >> On 6/13/17 1:22 AM, Oza Oza wrote: >> > On Tue, Jun 13, 2017 at 12:22 AM, Ray Jui <ray.jui@broadcom.com> wrote: >> >> >> >> >> >> On 6/12/17 10:40 AM, Oza Oza wrote: >> >>> On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi >> >>> <lorenzo.pieralisi@arm.com> wrote: >> >>>> On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: >> >>>>> On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi >> >>>>> <lorenzo.pieralisi@arm.com> wrote: >> >>>>>> [dropped rock-chips maintainers, email bounces] >> >>>>>> >> >>>>>> On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: >> >>>>>>> Hi Lorenzo, >> >>>>>>> >> >>>>>>> Thanks, I'll try my best to find time to test this along 15/42 and >> >>>>>>> 33/42 patches. Hopefully I can get to that some time next week. >> >>>>>>> >> >>>>>>> I have not yet reviewed these patches in details. Do they have >> >>>>>>> dependency on other patches to the generic framework code you >> >>>>>>> changed? >> >>>>>>> >> >>>>>>> If so, is there a repo I can pull them? >> >>>>>> >> >>>>>> I added it in the cover letter but anyway here it is: >> >>>>>> >> >>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 >> >>>>> >> >>>>> Hi Lorenzo, >> >>>>> >> >>>>> I picked up your changes, and tested on iproc based SOC, >> >>>>> and ran basic fio data transfer.. and it looks okay. >> >>>> >> >>>> Thank you. If you could also check it is all OK from a legacy >> >>>> IRQ allocation (ie same as before applying series) I'd be grateful. >> >>>> >> >>> >> >>> I disabled msi and it should default to legacy IRQ, looks like there >> >>> is a problem, if I missed any change ! >> >>> >> >>> before applying series: cat /proc/interrupts >> >>> 377: 168 0 0 0 0 0 >> >>> 0 0 dummy 1 Edge nvme0q0, nvme0q1 >> >>> >> >>> after applying series: >> >>> root@bcm958742k:~# dmesg | grep nvme >> >>> [ 3.855466] nvme 0000:01:00.0: assign irq: got 0 >> >>> [ 3.855469] nvme 0000:01:00.0: assigning IRQ 00 >> >>> [ 3.855515] nvme nvme0: pci function 0000:01:00.0 >> >>> [ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002) >> >>> [ 4.276787] nvme 0000:01:00.0: enabling bus mastering >> >>> [ 4.276817] nvme nvme0: Removing after probe failure status: -22 >> >>> >> >>> here is my git status of your series, let me know if I am missing any >> >>> change with respect to legacy IRQ ? >> >>> >> >>> modified: arch/arm/include/asm/mach/pci.h >> >>> modified: arch/arm/kernel/bios32.c >> >>> modified: arch/arm/mach-dove/pcie.c >> >>> modified: arch/arm/mach-iop13xx/pci.c >> >>> modified: arch/arm/mach-iop13xx/pci.h >> >>> modified: arch/arm/mach-mv78xx0/pcie.c >> >>> modified: arch/arm/mach-orion5x/common.h >> >>> modified: arch/arm/mach-orion5x/pci.c >> >>> modified: arch/arm64/kernel/pci.c >> >>> modified: drivers/of/of_pci_irq.c >> >>> modified: drivers/pci/Makefile >> >>> modified: drivers/pci/host/pci-aardvark.c >> >>> modified: drivers/pci/host/pci-ftpci100.c >> >>> modified: drivers/pci/host/pci-host-common.c >> >>> modified: drivers/pci/host/pci-tegra.c >> >>> modified: drivers/pci/host/pci-versatile.c >> >>> modified: drivers/pci/host/pci-xgene.c >> >>> modified: drivers/pci/host/pcie-altera.c >> >>> modified: drivers/pci/host/pcie-iproc-bcma.c >> >>> modified: drivers/pci/host/pcie-iproc-platform.c >> >>> modified: drivers/pci/host/pcie-iproc.c >> >>> modified: drivers/pci/host/pcie-iproc.h >> >>> modified: drivers/pci/host/pcie-rcar.c >> >>> modified: drivers/pci/host/pcie-rockchip.c >> >>> modified: drivers/pci/host/pcie-xilinx-nwl.c >> >>> modified: drivers/pci/host/pcie-xilinx.c >> >>> modified: drivers/pci/pci-driver.c >> >>> modified: drivers/pci/probe.c >> >>> modified: drivers/pci/setup-irq.c >> >>> modified: include/linux/pci.h >> >>> >> >> >> >> Did you test with or without my change in bcm/master to emulate legacy >> >> interrupt through dummy IRQ domain? >> >> >> >> Thanks, >> >> >> >> Ray >> >> >> >> >> > >> > Hi Ray, >> > yes irqdomain was missing, and with that legacy IRQ are fine now. >> > please mainline those changes. >> > >> > Hi Lorenzo, >> > >> > Legacy IRQ is working fine as well wiht your patches. >> > >> > cat /proc/interrupts >> > 122: 160 0 0 0 0 0 >> > 0 0 dummy 1 Edge nvme0q0, nvme0q1 >> > >> > NVMe data transfer is also fine. >> > >> > Regards, >> > Oza. >> > >> >> Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been >> sanity tested for link detection and legacy interrupt support. No >> regression is seen. Thanks, Oza. > > Yes, that's how I take it too, I did not understand what was triggering > the first regression reported - my series should work on top of mainline > if the current code works and should not depend on any other patch > being merged - please make sure that's the case to prevent unwanted > regressions in case other patches do not make it into the mainline. > > Thank you very much for your help in testing it. Hi Lorenzo, I have ported and tested my inbound memory patches on top of your patches and that work fine as well. This is mainly to do with IOVA reservations. Regards, Oza. > > Lorenzo > >> >> I'll send out the INTx irqdomain support patch when I have time. >> >> Ray >> >> >> >>
On Wed, Jun 21, 2017 at 8:09 PM, Oza Oza <oza.oza@broadcom.com> wrote: > On Wed, Jun 14, 2017 at 7:09 PM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: >> On Tue, Jun 13, 2017 at 10:18:14AM -0700, Ray Jui wrote: >>> Hi Oza/Lorenzo, >>> >>> On 6/13/17 1:22 AM, Oza Oza wrote: >>> > On Tue, Jun 13, 2017 at 12:22 AM, Ray Jui <ray.jui@broadcom.com> wrote: >>> >> >>> >> >>> >> On 6/12/17 10:40 AM, Oza Oza wrote: >>> >>> On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi >>> >>> <lorenzo.pieralisi@arm.com> wrote: >>> >>>> On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: >>> >>>>> On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi >>> >>>>> <lorenzo.pieralisi@arm.com> wrote: >>> >>>>>> [dropped rock-chips maintainers, email bounces] >>> >>>>>> >>> >>>>>> On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: >>> >>>>>>> Hi Lorenzo, >>> >>>>>>> >>> >>>>>>> Thanks, I'll try my best to find time to test this along 15/42 and >>> >>>>>>> 33/42 patches. Hopefully I can get to that some time next week. >>> >>>>>>> >>> >>>>>>> I have not yet reviewed these patches in details. Do they have >>> >>>>>>> dependency on other patches to the generic framework code you >>> >>>>>>> changed? >>> >>>>>>> >>> >>>>>>> If so, is there a repo I can pull them? >>> >>>>>> >>> >>>>>> I added it in the cover letter but anyway here it is: >>> >>>>>> >>> >>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 >>> >>>>> >>> >>>>> Hi Lorenzo, >>> >>>>> >>> >>>>> I picked up your changes, and tested on iproc based SOC, >>> >>>>> and ran basic fio data transfer.. and it looks okay. >>> >>>> >>> >>>> Thank you. If you could also check it is all OK from a legacy >>> >>>> IRQ allocation (ie same as before applying series) I'd be grateful. >>> >>>> >>> >>> >>> >>> I disabled msi and it should default to legacy IRQ, looks like there >>> >>> is a problem, if I missed any change ! >>> >>> >>> >>> before applying series: cat /proc/interrupts >>> >>> 377: 168 0 0 0 0 0 >>> >>> 0 0 dummy 1 Edge nvme0q0, nvme0q1 >>> >>> >>> >>> after applying series: >>> >>> root@bcm958742k:~# dmesg | grep nvme >>> >>> [ 3.855466] nvme 0000:01:00.0: assign irq: got 0 >>> >>> [ 3.855469] nvme 0000:01:00.0: assigning IRQ 00 >>> >>> [ 3.855515] nvme nvme0: pci function 0000:01:00.0 >>> >>> [ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002) >>> >>> [ 4.276787] nvme 0000:01:00.0: enabling bus mastering >>> >>> [ 4.276817] nvme nvme0: Removing after probe failure status: -22 >>> >>> >>> >>> here is my git status of your series, let me know if I am missing any >>> >>> change with respect to legacy IRQ ? >>> >>> >>> >>> modified: arch/arm/include/asm/mach/pci.h >>> >>> modified: arch/arm/kernel/bios32.c >>> >>> modified: arch/arm/mach-dove/pcie.c >>> >>> modified: arch/arm/mach-iop13xx/pci.c >>> >>> modified: arch/arm/mach-iop13xx/pci.h >>> >>> modified: arch/arm/mach-mv78xx0/pcie.c >>> >>> modified: arch/arm/mach-orion5x/common.h >>> >>> modified: arch/arm/mach-orion5x/pci.c >>> >>> modified: arch/arm64/kernel/pci.c >>> >>> modified: drivers/of/of_pci_irq.c >>> >>> modified: drivers/pci/Makefile >>> >>> modified: drivers/pci/host/pci-aardvark.c >>> >>> modified: drivers/pci/host/pci-ftpci100.c >>> >>> modified: drivers/pci/host/pci-host-common.c >>> >>> modified: drivers/pci/host/pci-tegra.c >>> >>> modified: drivers/pci/host/pci-versatile.c >>> >>> modified: drivers/pci/host/pci-xgene.c >>> >>> modified: drivers/pci/host/pcie-altera.c >>> >>> modified: drivers/pci/host/pcie-iproc-bcma.c >>> >>> modified: drivers/pci/host/pcie-iproc-platform.c >>> >>> modified: drivers/pci/host/pcie-iproc.c >>> >>> modified: drivers/pci/host/pcie-iproc.h >>> >>> modified: drivers/pci/host/pcie-rcar.c >>> >>> modified: drivers/pci/host/pcie-rockchip.c >>> >>> modified: drivers/pci/host/pcie-xilinx-nwl.c >>> >>> modified: drivers/pci/host/pcie-xilinx.c >>> >>> modified: drivers/pci/pci-driver.c >>> >>> modified: drivers/pci/probe.c >>> >>> modified: drivers/pci/setup-irq.c >>> >>> modified: include/linux/pci.h >>> >>> >>> >> >>> >> Did you test with or without my change in bcm/master to emulate legacy >>> >> interrupt through dummy IRQ domain? >>> >> >>> >> Thanks, >>> >> >>> >> Ray >>> >> >>> >> >>> > >>> > Hi Ray, >>> > yes irqdomain was missing, and with that legacy IRQ are fine now. >>> > please mainline those changes. >>> > >>> > Hi Lorenzo, >>> > >>> > Legacy IRQ is working fine as well wiht your patches. >>> > >>> > cat /proc/interrupts >>> > 122: 160 0 0 0 0 0 >>> > 0 0 dummy 1 Edge nvme0q0, nvme0q1 >>> > >>> > NVMe data transfer is also fine. >>> > >>> > Regards, >>> > Oza. >>> > >>> >>> Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been >>> sanity tested for link detection and legacy interrupt support. No >>> regression is seen. Thanks, Oza. >> >> Yes, that's how I take it too, I did not understand what was triggering >> the first regression reported - my series should work on top of mainline >> if the current code works and should not depend on any other patch >> being merged - please make sure that's the case to prevent unwanted >> regressions in case other patches do not make it into the mainline. >> >> Thank you very much for your help in testing it. > > > Hi Lorenzo, > > I have ported and tested my inbound memory patches on top of your > patches and that work fine as well. > This is mainly to do with IOVA reservations. > > Regards, > Oza. > >> >> Lorenzo >> >>> >>> I'll send out the INTx irqdomain support patch when I have time. >>> >>> Ray >>> >>> >>> >>> Hi Bjorn, I have made v8 for inbound memory patches as you suggested, on top of Lorenzo's patches. but I can not post them since Lorenzo's patches have not made in. Also gentle reminder to review following series [PATCH v5 1/2] PCI: iproc: Retry request when CRS returned from EP because that also conflict with Lorenzo's changes. Regards, Oza.
On Wed, Jul 19, 2017 at 05:43:32PM +0530, Oza Oza wrote: > On Wed, Jun 21, 2017 at 8:09 PM, Oza Oza <oza.oza@broadcom.com> wrote: > > On Wed, Jun 14, 2017 at 7:09 PM, Lorenzo Pieralisi > > <lorenzo.pieralisi@arm.com> wrote: > >> On Tue, Jun 13, 2017 at 10:18:14AM -0700, Ray Jui wrote: > >>> Hi Oza/Lorenzo, > >>> > >>> On 6/13/17 1:22 AM, Oza Oza wrote: > >>> > On Tue, Jun 13, 2017 at 12:22 AM, Ray Jui <ray.jui@broadcom.com> wrote: > >>> >> > >>> >> > >>> >> On 6/12/17 10:40 AM, Oza Oza wrote: > >>> >>> On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi > >>> >>> <lorenzo.pieralisi@arm.com> wrote: > >>> >>>> On Sun, Jun 11, 2017 at 09:42:34AM +0530, Oza Oza wrote: > >>> >>>>> On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi > >>> >>>>> <lorenzo.pieralisi@arm.com> wrote: > >>> >>>>>> [dropped rock-chips maintainers, email bounces] > >>> >>>>>> > >>> >>>>>> On Thu, Jun 08, 2017 at 08:56:05AM -0700, Ray Jui wrote: > >>> >>>>>>> Hi Lorenzo, > >>> >>>>>>> > >>> >>>>>>> Thanks, I'll try my best to find time to test this along 15/42 and > >>> >>>>>>> 33/42 patches. Hopefully I can get to that some time next week. > >>> >>>>>>> > >>> >>>>>>> I have not yet reviewed these patches in details. Do they have > >>> >>>>>>> dependency on other patches to the generic framework code you > >>> >>>>>>> changed? > >>> >>>>>>> > >>> >>>>>>> If so, is there a repo I can pull them? > >>> >>>>>> > >>> >>>>>> I added it in the cover letter but anyway here it is: > >>> >>>>>> > >>> >>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2 > >>> >>>>> > >>> >>>>> Hi Lorenzo, > >>> >>>>> > >>> >>>>> I picked up your changes, and tested on iproc based SOC, > >>> >>>>> and ran basic fio data transfer.. and it looks okay. > >>> >>>> > >>> >>>> Thank you. If you could also check it is all OK from a legacy > >>> >>>> IRQ allocation (ie same as before applying series) I'd be grateful. > >>> >>>> > >>> >>> > >>> >>> I disabled msi and it should default to legacy IRQ, looks like there > >>> >>> is a problem, if I missed any change ! > >>> >>> > >>> >>> before applying series: cat /proc/interrupts > >>> >>> 377: 168 0 0 0 0 0 > >>> >>> 0 0 dummy 1 Edge nvme0q0, nvme0q1 > >>> >>> > >>> >>> after applying series: > >>> >>> root@bcm958742k:~# dmesg | grep nvme > >>> >>> [ 3.855466] nvme 0000:01:00.0: assign irq: got 0 > >>> >>> [ 3.855469] nvme 0000:01:00.0: assigning IRQ 00 > >>> >>> [ 3.855515] nvme nvme0: pci function 0000:01:00.0 > >>> >>> [ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002) > >>> >>> [ 4.276787] nvme 0000:01:00.0: enabling bus mastering > >>> >>> [ 4.276817] nvme nvme0: Removing after probe failure status: -22 > >>> >>> > >>> >>> here is my git status of your series, let me know if I am missing any > >>> >>> change with respect to legacy IRQ ? > >>> >>> > >>> >>> modified: arch/arm/include/asm/mach/pci.h > >>> >>> modified: arch/arm/kernel/bios32.c > >>> >>> modified: arch/arm/mach-dove/pcie.c > >>> >>> modified: arch/arm/mach-iop13xx/pci.c > >>> >>> modified: arch/arm/mach-iop13xx/pci.h > >>> >>> modified: arch/arm/mach-mv78xx0/pcie.c > >>> >>> modified: arch/arm/mach-orion5x/common.h > >>> >>> modified: arch/arm/mach-orion5x/pci.c > >>> >>> modified: arch/arm64/kernel/pci.c > >>> >>> modified: drivers/of/of_pci_irq.c > >>> >>> modified: drivers/pci/Makefile > >>> >>> modified: drivers/pci/host/pci-aardvark.c > >>> >>> modified: drivers/pci/host/pci-ftpci100.c > >>> >>> modified: drivers/pci/host/pci-host-common.c > >>> >>> modified: drivers/pci/host/pci-tegra.c > >>> >>> modified: drivers/pci/host/pci-versatile.c > >>> >>> modified: drivers/pci/host/pci-xgene.c > >>> >>> modified: drivers/pci/host/pcie-altera.c > >>> >>> modified: drivers/pci/host/pcie-iproc-bcma.c > >>> >>> modified: drivers/pci/host/pcie-iproc-platform.c > >>> >>> modified: drivers/pci/host/pcie-iproc.c > >>> >>> modified: drivers/pci/host/pcie-iproc.h > >>> >>> modified: drivers/pci/host/pcie-rcar.c > >>> >>> modified: drivers/pci/host/pcie-rockchip.c > >>> >>> modified: drivers/pci/host/pcie-xilinx-nwl.c > >>> >>> modified: drivers/pci/host/pcie-xilinx.c > >>> >>> modified: drivers/pci/pci-driver.c > >>> >>> modified: drivers/pci/probe.c > >>> >>> modified: drivers/pci/setup-irq.c > >>> >>> modified: include/linux/pci.h > >>> >>> > >>> >> > >>> >> Did you test with or without my change in bcm/master to emulate legacy > >>> >> interrupt through dummy IRQ domain? > >>> >> > >>> >> Thanks, > >>> >> > >>> >> Ray > >>> >> > >>> >> > >>> > > >>> > Hi Ray, > >>> > yes irqdomain was missing, and with that legacy IRQ are fine now. > >>> > please mainline those changes. > >>> > > >>> > Hi Lorenzo, > >>> > > >>> > Legacy IRQ is working fine as well wiht your patches. > >>> > > >>> > cat /proc/interrupts > >>> > 122: 160 0 0 0 0 0 > >>> > 0 0 dummy 1 Edge nvme0q0, nvme0q1 > >>> > > >>> > NVMe data transfer is also fine. > >>> > > >>> > Regards, > >>> > Oza. > >>> > > >>> > >>> Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been > >>> sanity tested for link detection and legacy interrupt support. No > >>> regression is seen. Thanks, Oza. > >> > >> Yes, that's how I take it too, I did not understand what was triggering > >> the first regression reported - my series should work on top of mainline > >> if the current code works and should not depend on any other patch > >> being merged - please make sure that's the case to prevent unwanted > >> regressions in case other patches do not make it into the mainline. > >> > >> Thank you very much for your help in testing it. > > > > > > Hi Lorenzo, > > > > I have ported and tested my inbound memory patches on top of your > > patches and that work fine as well. > > This is mainly to do with IOVA reservations. > > > > Regards, > > Oza. > > > >> > >> Lorenzo > >> > >>> > >>> I'll send out the INTx irqdomain support patch when I have time. > >>> > >>> Ray > >>> > >>> > >>> > >>> > > Hi Bjorn, > > I have made v8 for inbound memory patches as you suggested, on top of > Lorenzo's patches. > but I can not post them since Lorenzo's patches have not made in. It depends on what kernel you are pulling. If you pulled v4.13-rc1 this series would be there. Lorenzo
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c index 0f39bd2..e48b8e2 100644 --- a/drivers/pci/host/pcie-iproc.c +++ b/drivers/pci/host/pcie-iproc.c @@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus, * Note access to the configuration registers are protected at the higher layer * by 'pci_lock' in drivers/pci/access.c */ -static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, +static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie, + int busno, unsigned int devfn, int where) { - struct iproc_pcie *pcie = iproc_data(bus); unsigned slot = PCI_SLOT(devfn); unsigned fn = PCI_FUNC(devfn); - unsigned busno = bus->number; u32 val; u16 offset; @@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus, return (pcie->base + offset); } +static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus, + unsigned int devfn, + int where) +{ + return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn, + where); +} + +static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie, + unsigned int devfn, int where, + int size, u32 *val) +{ + void __iomem *addr; + + addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); + if (!addr) { + *val = ~0; + return PCIBIOS_DEVICE_NOT_FOUND; + } + + *val = readl(addr); + + if (size <= 2) + *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1); + + return PCIBIOS_SUCCESSFUL; +} + +static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie, + unsigned int devfn, int where, + int size, u32 val) +{ + void __iomem *addr; + u32 mask, tmp; + + addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3); + if (!addr) + return PCIBIOS_DEVICE_NOT_FOUND; + + if (size == 4) { + writel(val, addr); + return PCIBIOS_SUCCESSFUL; + } + + mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8)); + tmp = readl(addr) & mask; + tmp |= val << ((where & 0x3) * 8); + writel(tmp, addr); + + return PCIBIOS_SUCCESSFUL; +} + static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val) { @@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn, } static struct pci_ops iproc_pcie_ops = { - .map_bus = iproc_pcie_map_cfg_bus, + .map_bus = iproc_pcie_bus_map_cfg_bus, .read = iproc_pcie_config_read32, .write = iproc_pcie_config_write32, }; @@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie) msleep(100); } -static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) +static int iproc_pcie_check_link(struct iproc_pcie *pcie) { struct device *dev = pcie->dev; - u8 hdr_type; - u32 link_ctrl, class, val; - u16 pos = PCI_EXP_CAP, link_status; + u32 hdr_type, link_ctrl, link_status, class, val; + u16 pos = PCI_EXP_CAP; bool link_is_active = false; /* @@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) } /* make sure we are not in EP mode */ - pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type); + iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type); if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) { dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type); return -EFAULT; @@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c #define PCI_CLASS_BRIDGE_MASK 0xffff00 #define PCI_CLASS_BRIDGE_SHIFT 8 - pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class); + iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, + 4, &class); class &= ~PCI_CLASS_BRIDGE_MASK; class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT); - pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class); + iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET, + 4, class); /* check link status to see if link is active */ - pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status); + iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA, + 2, &link_status); if (link_status & PCI_EXP_LNKSTA_NLW) link_is_active = true; @@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus) #define PCI_TARGET_LINK_SPEED_MASK 0xf #define PCI_TARGET_LINK_SPEED_GEN2 0x2 #define PCI_TARGET_LINK_SPEED_GEN1 0x1 - pci_bus_read_config_dword(bus, 0, - pos + PCI_EXP_LNKCTL2, + iproc_pci_raw_config_read32(pcie, 0, + pos + PCI_EXP_LNKCTL2, 4, &link_ctrl); if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) == PCI_TARGET_LINK_SPEED_GEN2) { link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK; link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1; - pci_bus_write_config_dword(bus, 0, - pos + PCI_EXP_LNKCTL2, - link_ctrl); + iproc_pci_raw_config_write32(pcie, 0, + pos + PCI_EXP_LNKCTL2, + 4, link_ctrl); msleep(100); - pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, - &link_status); + iproc_pci_raw_config_read32(pcie, 0, + pos + PCI_EXP_LNKSTA, + 2, &link_status); if (link_status & PCI_EXP_LNKSTA_NLW) link_is_active = true; } @@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) } pcie->root_bus = bus; - ret = iproc_pcie_check_link(pcie, bus); + ret = iproc_pcie_check_link(pcie); if (ret) { dev_err(dev, "no PCIe EP device detected\n"); goto err_rm_root_bus;
Current iproc driver host bridge controller driver requires struct pci_bus to be created in order to carry out PCI link checks with standard PCI config space accessors. This struct pci_bus dependency is fictitious and burdens the driver with unneeded constraints (eg to use separate APIs to create and scan the root bus). Add PCI raw config space accessors to PCIe iproc driver and remove the fictitious struct pci_bus dependency. Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Scott Branden <sbranden@broadcom.com> Cc: Ray Jui <rjui@broadcom.com> Cc: Jon Mason <jonmason@broadcom.com> --- drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 74 insertions(+), 20 deletions(-)