| Message ID | 1585206447-1363-3-git-send-email-srinath.mannam@broadcom.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | PCI: iproc: Add fixes to pcie iproc | expand |
[fixed Andrew's email addr] Change subject to match convention, e.g., PCI: iproc: Invalidate correct PAXB inbound windows On Thu, Mar 26, 2020 at 12:37:26PM +0530, Srinath Mannam wrote: > From: Roman Bacik <roman.bacik@broadcom.com> > > Second stage bootloader prior to Linux boot may use all inbound windows > including IARR1/IMAP1. We need to ensure all previous configuration of > inbound windows are invalidated during the initialization stage of the > Linux iProc PCIe driver. Add fix to invalidate IARR1/IMAP1 because it was > missed in previous patch. > > Fixes: 9415743e4c8a ("PCI: iproc: Invalidate PAXB address mapping") > Signed-off-by: Roman Bacik <roman.bacik@broadcom.com> > --- > drivers/pci/controller/pcie-iproc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c > index 6972ca4..e7f0d58 100644 > --- a/drivers/pci/controller/pcie-iproc.c > +++ b/drivers/pci/controller/pcie-iproc.c > @@ -351,6 +351,8 @@ static const u16 iproc_pcie_reg_paxb_v2[IPROC_PCIE_MAX_NUM_REG] = { > [IPROC_PCIE_OMAP3] = 0xdf8, > [IPROC_PCIE_IARR0] = 0xd00, > [IPROC_PCIE_IMAP0] = 0xc00, > + [IPROC_PCIE_IARR1] = 0xd08, > + [IPROC_PCIE_IMAP1] = 0xd70, Wow, this is a little too subtle for my taste. 9415743e4c8a added this loop: + for (idx = 0; idx < ib->nr_regions; idx++) { + iproc_pcie_write_reg(pcie, + MAP_REG(IPROC_PCIE_IARR0, idx), 0); This patch doesn't change the loop or the limit, so I *guess* previously we invalidated IPROC_PCIE_IARR0, IPROC_PCIE_IARR2, ..., (skipping IPROC_PCIE_IARR1), and now we will invalidate IPROC_PCIE_IARR0, IPROC_PCIE_IARR1, ... instead? > [IPROC_PCIE_IARR2] = 0xd10, > [IPROC_PCIE_IMAP2] = 0xcc0, > [IPROC_PCIE_IARR3] = 0xe00,
On Thu, Mar 26, 2020 at 12:37:26PM +0530, Srinath Mannam wrote: > From: Roman Bacik <roman.bacik@broadcom.com> > > Second stage bootloader prior to Linux boot may use all inbound windows > including IARR1/IMAP1. We need to ensure all previous configuration of > inbound windows are invalidated during the initialization stage of the > Linux iProc PCIe driver. Add fix to invalidate IARR1/IMAP1 because it was > missed in previous patch. > > Fixes: 9415743e4c8a ("PCI: iproc: Invalidate PAXB address mapping") > Signed-off-by: Roman Bacik <roman.bacik@broadcom.com> > --- > drivers/pci/controller/pcie-iproc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c > index 6972ca4..e7f0d58 100644 > --- a/drivers/pci/controller/pcie-iproc.c > +++ b/drivers/pci/controller/pcie-iproc.c > @@ -351,6 +351,8 @@ static const u16 iproc_pcie_reg_paxb_v2[IPROC_PCIE_MAX_NUM_REG] = { > [IPROC_PCIE_OMAP3] = 0xdf8, > [IPROC_PCIE_IARR0] = 0xd00, > [IPROC_PCIE_IMAP0] = 0xc00, > + [IPROC_PCIE_IARR1] = 0xd08, > + [IPROC_PCIE_IMAP1] = 0xd70, And paxb_v2_ib_map[] has a comment saying "IARR1/IMAP1 (currently unused)". Is that comment now wrong? > [IPROC_PCIE_IARR2] = 0xd10, > [IPROC_PCIE_IMAP2] = 0xcc0, > [IPROC_PCIE_IARR3] = 0xe00, > -- > 2.7.4 >
On Thu, Mar 26, 2020 at 8:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Thu, Mar 26, 2020 at 12:37:26PM +0530, Srinath Mannam wrote: > > From: Roman Bacik <roman.bacik@broadcom.com> > > > > Second stage bootloader prior to Linux boot may use all inbound windows > > including IARR1/IMAP1. We need to ensure all previous configuration of > > inbound windows are invalidated during the initialization stage of the > > Linux iProc PCIe driver. Add fix to invalidate IARR1/IMAP1 because it was > > missed in previous patch. > > > > Fixes: 9415743e4c8a ("PCI: iproc: Invalidate PAXB address mapping") > > Signed-off-by: Roman Bacik <roman.bacik@broadcom.com> > > --- > > drivers/pci/controller/pcie-iproc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c > > index 6972ca4..e7f0d58 100644 > > --- a/drivers/pci/controller/pcie-iproc.c > > +++ b/drivers/pci/controller/pcie-iproc.c > > @@ -351,6 +351,8 @@ static const u16 iproc_pcie_reg_paxb_v2[IPROC_PCIE_MAX_NUM_REG] = { > > [IPROC_PCIE_OMAP3] = 0xdf8, > > [IPROC_PCIE_IARR0] = 0xd00, > > [IPROC_PCIE_IMAP0] = 0xc00, > > + [IPROC_PCIE_IARR1] = 0xd08, > > + [IPROC_PCIE_IMAP1] = 0xd70, > > And paxb_v2_ib_map[] has a comment saying "IARR1/IMAP1 (currently > unused)". Is that comment now wrong? > The comment is still correct, IARR1/IMAP1 is unused in Linux. But it may need to be invalidated in case it was modified by bootloaders. > > [IPROC_PCIE_IARR2] = 0xd10, > > [IPROC_PCIE_IMAP2] = 0xcc0, > > [IPROC_PCIE_IARR3] = 0xe00, > > -- > > 2.7.4 > >
diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c index 6972ca4..e7f0d58 100644 --- a/drivers/pci/controller/pcie-iproc.c +++ b/drivers/pci/controller/pcie-iproc.c @@ -351,6 +351,8 @@ static const u16 iproc_pcie_reg_paxb_v2[IPROC_PCIE_MAX_NUM_REG] = { [IPROC_PCIE_OMAP3] = 0xdf8, [IPROC_PCIE_IARR0] = 0xd00, [IPROC_PCIE_IMAP0] = 0xc00, + [IPROC_PCIE_IARR1] = 0xd08, + [IPROC_PCIE_IMAP1] = 0xd70, [IPROC_PCIE_IARR2] = 0xd10, [IPROC_PCIE_IMAP2] = 0xcc0, [IPROC_PCIE_IARR3] = 0xe00,