| Message ID | 1441817460-174811-1-git-send-email-gabriele.paoloni@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Wed, Sep 9, 2015 at 10:21 PM, Gabriele Paoloni <gabriele.paoloni@huawei.com> wrote: > int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val) > { > + addr += (where & ~0x3); Lets see what Jingoo and other have to say on it. IMO, where should be better kept as an offset within 32 bit word and addr should be 32 bit aligned. I think this is how where has been considered at other places like pci-mvebu.c. > *val = readl(addr); > - > - if (size == 1) > - *val = (*val >> (8 * (where & 3))) & 0xff; > - else if (size == 2) > - *val = (*val >> (8 * (where & 3))) & 0xffff; > - else if (size != 4) > + where &= 3; > + > + if (size == 1) { > + *val = (*val >> (8 * where)) & 0xff; > + } else if (size == 2) { > + if (where & 1) > + return PCIBIOS_BAD_REGISTER_NUMBER; Do we really need such cross check? I do not think that any caller is expected for such misbehave. ~Pratyush -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Gabriele, On Wed, Sep 9, 2015 at 11:51 AM, Gabriele Paoloni <gabriele.paoloni@huawei.com> wrote: > From: gabriele paoloni <gabriele.paoloni@huawei.com> > > This patch changes the implementation of dw_pcie_cfg_read() and > dw_pcie_cfg_write() to: > - improve the function usage from the callers perspective; > - make sure that the callers specify proper address offsets > according to the size of the field being read/written > - fix a bug in pcie-spear13xx.c where the callers where passing > the wrong address as they missed to add the appropriate offset Please split this into separate patches, so each patch does one thing, i.e., separate the "where" usage change from the "return PCIBIOS_BAD_REGISTER_NUMBER for bad offsets" change. I think there was a separate patch for the pcie-spear13xx.c bug, wasn't there? At least, I don't see that fix in *this* patch. > This patch is a follow up from the discussion in: > http://www.spinics.net/lists/linux-pci/msg44351.html > > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> > --- > drivers/pci/host/pci-exynos.c | 5 ++-- > drivers/pci/host/pci-keystone-dw.c | 4 +-- > drivers/pci/host/pcie-designware.c | 51 ++++++++++++++++++++++---------------- > 3 files changed, 34 insertions(+), 26 deletions(-) > > diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c > index f9f468d..8b0e04b 100644 > --- a/drivers/pci/host/pci-exynos.c > +++ b/drivers/pci/host/pci-exynos.c > @@ -454,7 +454,7 @@ static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size, > int ret; > > exynos_pcie_sideband_dbi_r_mode(pp, true); > - ret = dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where, size, val); > + ret = dw_pcie_cfg_read(pp->dbi_base, where, size, val); > exynos_pcie_sideband_dbi_r_mode(pp, false); > return ret; > } > @@ -465,8 +465,7 @@ static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size, > int ret; > > exynos_pcie_sideband_dbi_w_mode(pp, true); > - ret = dw_pcie_cfg_write(pp->dbi_base + (where & ~0x3), > - where, size, val); > + ret = dw_pcie_cfg_write(pp->dbi_base, where, size, val); > exynos_pcie_sideband_dbi_w_mode(pp, false); > return ret; > } > diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c > index f34892e..2b391f4 100644 > --- a/drivers/pci/host/pci-keystone-dw.c > +++ b/drivers/pci/host/pci-keystone-dw.c > @@ -403,7 +403,7 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, > > addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn); > > - return dw_pcie_cfg_read(addr + (where & ~0x3), where, size, val); > + return dw_pcie_cfg_read(addr, where, size, val); > } > > int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, > @@ -415,7 +415,7 @@ int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, > > addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn); > > - return dw_pcie_cfg_write(addr + (where & ~0x3), where, size, val); > + return dw_pcie_cfg_write(addr, where, size, val); > } > > /** > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index 69486be..b53aa38 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -82,13 +82,20 @@ static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys) > > int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val) > { > + addr += (where & ~0x3); > *val = readl(addr); > - > - if (size == 1) > - *val = (*val >> (8 * (where & 3))) & 0xff; > - else if (size == 2) > - *val = (*val >> (8 * (where & 3))) & 0xffff; > - else if (size != 4) > + where &= 3; > + > + if (size == 1) { > + *val = (*val >> (8 * where)) & 0xff; > + } else if (size == 2) { > + if (where & 1) > + return PCIBIOS_BAD_REGISTER_NUMBER; > + *val = (*val >> (8 * where)) & 0xffff; > + } else if (size == 4) { > + if (where & 3) > + return PCIBIOS_BAD_REGISTER_NUMBER; > + } else > return PCIBIOS_BAD_REGISTER_NUMBER; > > return PCIBIOS_SUCCESSFUL; > @@ -96,12 +103,18 @@ int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val) > > int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 val) > { > - if (size == 4) > + addr += where; > + > + if (size == 4) { > + if ((uintptr_t)addr & 3) > + return PCIBIOS_BAD_REGISTER_NUMBER; > writel(val, addr); > - else if (size == 2) > - writew(val, addr + (where & 2)); > - else if (size == 1) > - writeb(val, addr + (where & 3)); > + } else if (size == 2) { > + if ((uintptr_t)addr & 1) > + return PCIBIOS_BAD_REGISTER_NUMBER; > + writew(val, addr); > + } else if (size == 1) > + writeb(val, addr); > else > return PCIBIOS_BAD_REGISTER_NUMBER; > > @@ -132,8 +145,7 @@ static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size, > if (pp->ops->rd_own_conf) > ret = pp->ops->rd_own_conf(pp, where, size, val); > else > - ret = dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where, > - size, val); > + ret = dw_pcie_cfg_read(pp->dbi_base, where, size, val); > > return ret; > } > @@ -146,8 +158,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size, > if (pp->ops->wr_own_conf) > ret = pp->ops->wr_own_conf(pp, where, size, val); > else > - ret = dw_pcie_cfg_write(pp->dbi_base + (where & ~0x3), where, > - size, val); > + ret = dw_pcie_cfg_write(pp->dbi_base, where, size, val); > > return ret; > } > @@ -541,13 +552,12 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, > u32 devfn, int where, int size, u32 *val) > { > int ret, type; > - u32 address, busdev, cfg_size; > + u32 busdev, cfg_size; > u64 cpu_addr; > void __iomem *va_cfg_base; > > busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | > PCIE_ATU_FUNC(PCI_FUNC(devfn)); > - address = where & ~0x3; > > if (bus->parent->number == pp->root_bus_nr) { > type = PCIE_ATU_TYPE_CFG0; > @@ -564,7 +574,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > type, cpu_addr, > busdev, cfg_size); > - ret = dw_pcie_cfg_read(va_cfg_base + address, where, size, val); > + ret = dw_pcie_cfg_read(va_cfg_base, where, size, val); > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > PCIE_ATU_TYPE_IO, pp->io_mod_base, > pp->io_bus_addr, pp->io_size); > @@ -576,13 +586,12 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, > u32 devfn, int where, int size, u32 val) > { > int ret, type; > - u32 address, busdev, cfg_size; > + u32 busdev, cfg_size; > u64 cpu_addr; > void __iomem *va_cfg_base; > > busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | > PCIE_ATU_FUNC(PCI_FUNC(devfn)); > - address = where & ~0x3; > > if (bus->parent->number == pp->root_bus_nr) { > type = PCIE_ATU_TYPE_CFG0; > @@ -599,7 +608,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > type, cpu_addr, > busdev, cfg_size); > - ret = dw_pcie_cfg_write(va_cfg_base + address, where, size, val); > + ret = dw_pcie_cfg_write(va_cfg_base, where, size, val); > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > PCIE_ATU_TYPE_IO, pp->io_mod_base, > pp->io_bus_addr, pp->io_size); > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, thanks for your comments > -----Original Message----- > From: Bjorn Helgaas [mailto:bhelgaas@google.com] > Sent: Wednesday, September 09, 2015 8:45 PM > To: Gabriele Paoloni > Cc: Jingoo Han; Pratyush Anand Thakur; linux-pci@vger.kernel.org; > Wangzhou (B); Yuanzhichang; Zhudacai; zhangjukuo; qiuzhenfa; Liguozhu > (Kenneth) > Subject: Re: [PATCH] PCI: designware: change dw_pcie_cfg_write() and > dw_pcie_cfg_read() > > Hi Gabriele, > > On Wed, Sep 9, 2015 at 11:51 AM, Gabriele Paoloni > <gabriele.paoloni@huawei.com> wrote: > > From: gabriele paoloni <gabriele.paoloni@huawei.com> > > > > This patch changes the implementation of dw_pcie_cfg_read() and > > dw_pcie_cfg_write() to: > > - improve the function usage from the callers perspective; > > - make sure that the callers specify proper address offsets > > according to the size of the field being read/written > > - fix a bug in pcie-spear13xx.c where the callers where passing > > the wrong address as they missed to add the appropriate offset > > Please split this into separate patches, so each patch does one thing, > i.e., separate the "where" usage change from the "return > PCIBIOS_BAD_REGISTER_NUMBER for bad offsets" change. Ok I will send out a patchset with separate patches > > I think there was a separate patch for the pcie-spear13xx.c bug, > wasn't there? At least, I don't see that fix in *this* patch. > From your previous comments: ******** *2) Pratyush, this one's for you :) spear13xx_pcie_establish_link() * calls dw_pcie_cfg_read() and dw_pcie_cfg_write() several times like * this: * * dw_pcie_cfg_read(pp->dbi_base, exp_cap_off + PCI_EXP_DEVCTL, 4, ...) * dw_pcie_cfg_write(pp->dbi_base, exp_cap_off + PCI_EXP_DEVCTL, 4, ...) ******** I think that the implementation of dw_pcie_cfg_read() and dw_pcie_cfg_write() that I proposed in this patch is compatible with the previous calls in pcie-spear13xx.c. So the calls in pcie-spear13xx.c were buggy for the previous implementations of the functions; with the reworked functions there is no need to modify the function calls in pcie-spear13xx.c...do you agree? > > This patch is a follow up from the discussion in: > > http://www.spinics.net/lists/linux-pci/msg44351.html > > > > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> > > --- > > drivers/pci/host/pci-exynos.c | 5 ++-- > > drivers/pci/host/pci-keystone-dw.c | 4 +-- > > drivers/pci/host/pcie-designware.c | 51 ++++++++++++++++++++++------ > ---------- > > 3 files changed, 34 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci- > exynos.c > > index f9f468d..8b0e04b 100644 > > --- a/drivers/pci/host/pci-exynos.c > > +++ b/drivers/pci/host/pci-exynos.c > > @@ -454,7 +454,7 @@ static int exynos_pcie_rd_own_conf(struct > pcie_port *pp, int where, int size, > > int ret; > > > > exynos_pcie_sideband_dbi_r_mode(pp, true); > > - ret = dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where, > size, val); > > + ret = dw_pcie_cfg_read(pp->dbi_base, where, size, val); > > exynos_pcie_sideband_dbi_r_mode(pp, false); > > return ret; > > } > > @@ -465,8 +465,7 @@ static int exynos_pcie_wr_own_conf(struct > pcie_port *pp, int where, int size, > > int ret; > > > > exynos_pcie_sideband_dbi_w_mode(pp, true); > > - ret = dw_pcie_cfg_write(pp->dbi_base + (where & ~0x3), > > - where, size, val); > > + ret = dw_pcie_cfg_write(pp->dbi_base, where, size, val); > > exynos_pcie_sideband_dbi_w_mode(pp, false); > > return ret; > > } > > diff --git a/drivers/pci/host/pci-keystone-dw.c > b/drivers/pci/host/pci-keystone-dw.c > > index f34892e..2b391f4 100644 > > --- a/drivers/pci/host/pci-keystone-dw.c > > +++ b/drivers/pci/host/pci-keystone-dw.c > > @@ -403,7 +403,7 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, > struct pci_bus *bus, > > > > addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn); > > > > - return dw_pcie_cfg_read(addr + (where & ~0x3), where, size, > val); > > + return dw_pcie_cfg_read(addr, where, size, val); > > } > > > > int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus > *bus, > > @@ -415,7 +415,7 @@ int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, > struct pci_bus *bus, > > > > addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn); > > > > - return dw_pcie_cfg_write(addr + (where & ~0x3), where, size, > val); > > + return dw_pcie_cfg_write(addr, where, size, val); > > } > > > > /** > > diff --git a/drivers/pci/host/pcie-designware.c > b/drivers/pci/host/pcie-designware.c > > index 69486be..b53aa38 100644 > > --- a/drivers/pci/host/pcie-designware.c > > +++ b/drivers/pci/host/pcie-designware.c > > @@ -82,13 +82,20 @@ static inline struct pcie_port > *sys_to_pcie(struct pci_sys_data *sys) > > > > int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 > *val) > > { > > + addr += (where & ~0x3); > > *val = readl(addr); > > - > > - if (size == 1) > > - *val = (*val >> (8 * (where & 3))) & 0xff; > > - else if (size == 2) > > - *val = (*val >> (8 * (where & 3))) & 0xffff; > > - else if (size != 4) > > + where &= 3; > > + > > + if (size == 1) { > > + *val = (*val >> (8 * where)) & 0xff; > > + } else if (size == 2) { > > + if (where & 1) > > + return PCIBIOS_BAD_REGISTER_NUMBER; > > + *val = (*val >> (8 * where)) & 0xffff; > > + } else if (size == 4) { > > + if (where & 3) > > + return PCIBIOS_BAD_REGISTER_NUMBER; > > + } else > > return PCIBIOS_BAD_REGISTER_NUMBER; > > > > return PCIBIOS_SUCCESSFUL; > > @@ -96,12 +103,18 @@ int dw_pcie_cfg_read(void __iomem *addr, int > where, int size, u32 *val) > > > > int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 > val) > > { > > - if (size == 4) > > + addr += where; > > + > > + if (size == 4) { > > + if ((uintptr_t)addr & 3) > > + return PCIBIOS_BAD_REGISTER_NUMBER; > > writel(val, addr); > > - else if (size == 2) > > - writew(val, addr + (where & 2)); > > - else if (size == 1) > > - writeb(val, addr + (where & 3)); > > + } else if (size == 2) { > > + if ((uintptr_t)addr & 1) > > + return PCIBIOS_BAD_REGISTER_NUMBER; > > + writew(val, addr); > > + } else if (size == 1) > > + writeb(val, addr); > > else > > return PCIBIOS_BAD_REGISTER_NUMBER; > > > > @@ -132,8 +145,7 @@ static int dw_pcie_rd_own_conf(struct pcie_port > *pp, int where, int size, > > if (pp->ops->rd_own_conf) > > ret = pp->ops->rd_own_conf(pp, where, size, val); > > else > > - ret = dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), > where, > > - size, val); > > + ret = dw_pcie_cfg_read(pp->dbi_base, where, size, > val); > > > > return ret; > > } > > @@ -146,8 +158,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port > *pp, int where, int size, > > if (pp->ops->wr_own_conf) > > ret = pp->ops->wr_own_conf(pp, where, size, val); > > else > > - ret = dw_pcie_cfg_write(pp->dbi_base + (where & ~0x3), > where, > > - size, val); > > + ret = dw_pcie_cfg_write(pp->dbi_base, where, size, > val); > > > > return ret; > > } > > @@ -541,13 +552,12 @@ static int dw_pcie_rd_other_conf(struct > pcie_port *pp, struct pci_bus *bus, > > u32 devfn, int where, int size, u32 *val) > > { > > int ret, type; > > - u32 address, busdev, cfg_size; > > + u32 busdev, cfg_size; > > u64 cpu_addr; > > void __iomem *va_cfg_base; > > > > busdev = PCIE_ATU_BUS(bus->number) | > PCIE_ATU_DEV(PCI_SLOT(devfn)) | > > PCIE_ATU_FUNC(PCI_FUNC(devfn)); > > - address = where & ~0x3; > > > > if (bus->parent->number == pp->root_bus_nr) { > > type = PCIE_ATU_TYPE_CFG0; > > @@ -564,7 +574,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port > *pp, struct pci_bus *bus, > > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > > type, cpu_addr, > > busdev, cfg_size); > > - ret = dw_pcie_cfg_read(va_cfg_base + address, where, size, > val); > > + ret = dw_pcie_cfg_read(va_cfg_base, where, size, val); > > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > > PCIE_ATU_TYPE_IO, pp->io_mod_base, > > pp->io_bus_addr, pp->io_size); > > @@ -576,13 +586,12 @@ static int dw_pcie_wr_other_conf(struct > pcie_port *pp, struct pci_bus *bus, > > u32 devfn, int where, int size, u32 val) > > { > > int ret, type; > > - u32 address, busdev, cfg_size; > > + u32 busdev, cfg_size; > > u64 cpu_addr; > > void __iomem *va_cfg_base; > > > > busdev = PCIE_ATU_BUS(bus->number) | > PCIE_ATU_DEV(PCI_SLOT(devfn)) | > > PCIE_ATU_FUNC(PCI_FUNC(devfn)); > > - address = where & ~0x3; > > > > if (bus->parent->number == pp->root_bus_nr) { > > type = PCIE_ATU_TYPE_CFG0; > > @@ -599,7 +608,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port > *pp, struct pci_bus *bus, > > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > > type, cpu_addr, > > busdev, cfg_size); > > - ret = dw_pcie_cfg_write(va_cfg_base + address, where, size, > val); > > + ret = dw_pcie_cfg_write(va_cfg_base, where, size, val); > > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > > PCIE_ATU_TYPE_IO, pp->io_mod_base, > > pp->io_bus_addr, pp->io_size); > > -- > > 1.9.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-pci" > in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 10, 2015 at 6:26 AM, Gabriele Paoloni <gabriele.paoloni@huawei.com> wrote: >> From: Bjorn Helgaas [mailto:bhelgaas@google.com] >> I think there was a separate patch for the pcie-spear13xx.c bug, >> wasn't there? At least, I don't see that fix in *this* patch. >> > > From your previous comments: > ******** > *2) Pratyush, this one's for you :) spear13xx_pcie_establish_link() > * calls dw_pcie_cfg_read() and dw_pcie_cfg_write() several times like > * this: > * > * dw_pcie_cfg_read(pp->dbi_base, exp_cap_off + PCI_EXP_DEVCTL, 4, ...) > * dw_pcie_cfg_write(pp->dbi_base, exp_cap_off + PCI_EXP_DEVCTL, 4, ...) > ******** > > I think that the implementation of dw_pcie_cfg_read() and > dw_pcie_cfg_write() that I proposed in this patch is compatible with > the previous calls in pcie-spear13xx.c. So the calls in pcie-spear13xx.c > were buggy for the previous implementations of the functions; with > the reworked functions there is no need to modify the function calls > in pcie-spear13xx.c...do you agree? Oh, I see, I didn't read it closely enough, but you're probably right. What I would probably do is apply Pratyush's fix first, then apply your patch on top. That way we can do the spear13xx bug fix by itself (which should be a fairly obviously-correct patch), then your interface change (which would be extended to touch all the calls including spear13xx and be similarly obviously correct). I think that would make it a bit easier for future changelog archaeologists. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmpvcm4gSGVsZ2FhcyBb bWFpbHRvOmJoZWxnYWFzQGdvb2dsZS5jb21dDQo+IFNlbnQ6IFRodXJzZGF5LCBTZXB0ZW1iZXIg MTAsIDIwMTUgMTo0MCBQTQ0KPiBUbzogR2FicmllbGUgUGFvbG9uaQ0KPiBDYzogSmluZ29vIEhh bjsgUHJhdHl1c2ggQW5hbmQgVGhha3VyOyBsaW51eC1wY2lAdmdlci5rZXJuZWwub3JnOw0KPiBX YW5nemhvdSAoQik7IFl1YW56aGljaGFuZzsgWmh1ZGFjYWk7IHpoYW5nanVrdW87IHFpdXpoZW5m YTsgTGlndW96aHUNCj4gKEtlbm5ldGgpDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0hdIFBDSTogZGVz aWdud2FyZTogY2hhbmdlIGR3X3BjaWVfY2ZnX3dyaXRlKCkgYW5kDQo+IGR3X3BjaWVfY2ZnX3Jl YWQoKQ0KPiANCj4gT24gVGh1LCBTZXAgMTAsIDIwMTUgYXQgNjoyNiBBTSwgR2FicmllbGUgUGFv bG9uaQ0KPiA8Z2FicmllbGUucGFvbG9uaUBodWF3ZWkuY29tPiB3cm90ZToNCj4gPj4gRnJvbTog Qmpvcm4gSGVsZ2FhcyBbbWFpbHRvOmJoZWxnYWFzQGdvb2dsZS5jb21dDQo+IA0KPiA+PiBJIHRo aW5rIHRoZXJlIHdhcyBhIHNlcGFyYXRlIHBhdGNoIGZvciB0aGUgcGNpZS1zcGVhcjEzeHguYyBi dWcsDQo+ID4+IHdhc24ndCB0aGVyZT8gIEF0IGxlYXN0LCBJIGRvbid0IHNlZSB0aGF0IGZpeCBp biAqdGhpcyogcGF0Y2guDQo+ID4+DQo+ID4NCj4gPiBGcm9tIHlvdXIgcHJldmlvdXMgY29tbWVu dHM6DQo+ID4gKioqKioqKioNCj4gPiAqMikgUHJhdHl1c2gsIHRoaXMgb25lJ3MgZm9yIHlvdSA6 KSAgc3BlYXIxM3h4X3BjaWVfZXN0YWJsaXNoX2xpbmsoKQ0KPiA+ICogICBjYWxscyBkd19wY2ll X2NmZ19yZWFkKCkgYW5kIGR3X3BjaWVfY2ZnX3dyaXRlKCkgc2V2ZXJhbCB0aW1lcw0KPiBsaWtl DQo+ID4gKiAgIHRoaXM6DQo+ID4gKg0KPiA+ICogICAgZHdfcGNpZV9jZmdfcmVhZChwcC0+ZGJp X2Jhc2UsIGV4cF9jYXBfb2ZmICsgUENJX0VYUF9ERVZDVEwsDQo+IDQsIC4uLikNCj4gPiAqICAg IGR3X3BjaWVfY2ZnX3dyaXRlKHBwLT5kYmlfYmFzZSwgZXhwX2NhcF9vZmYgKyBQQ0lfRVhQX0RF VkNUTCwNCj4gNCwgLi4uKQ0KPiA+ICoqKioqKioqDQo+ID4NCj4gPiBJIHRoaW5rIHRoYXQgdGhl IGltcGxlbWVudGF0aW9uIG9mIGR3X3BjaWVfY2ZnX3JlYWQoKSBhbmQNCj4gPiBkd19wY2llX2Nm Z193cml0ZSgpIHRoYXQgSSBwcm9wb3NlZCBpbiB0aGlzIHBhdGNoIGlzIGNvbXBhdGlibGUgd2l0 aA0KPiA+IHRoZSBwcmV2aW91cyBjYWxscyBpbiBwY2llLXNwZWFyMTN4eC5jLiBTbyB0aGUgY2Fs bHMgaW4gcGNpZS0NCj4gc3BlYXIxM3h4LmMNCj4gPiB3ZXJlIGJ1Z2d5IGZvciB0aGUgcHJldmlv dXMgaW1wbGVtZW50YXRpb25zIG9mIHRoZSBmdW5jdGlvbnM7IHdpdGgNCj4gPiB0aGUgcmV3b3Jr ZWQgZnVuY3Rpb25zIHRoZXJlIGlzIG5vIG5lZWQgdG8gbW9kaWZ5IHRoZSBmdW5jdGlvbiBjYWxs cw0KPiA+IGluIHBjaWUtc3BlYXIxM3h4LmMuLi5kbyB5b3UgYWdyZWU/DQo+IA0KPiBPaCwgSSBz ZWUsIEkgZGlkbid0IHJlYWQgaXQgY2xvc2VseSBlbm91Z2gsIGJ1dCB5b3UncmUgcHJvYmFibHkg cmlnaHQuDQo+IFdoYXQgSSB3b3VsZCBwcm9iYWJseSBkbyBpcyBhcHBseSBQcmF0eXVzaCdzIGZp eCBmaXJzdCwgdGhlbiBhcHBseQ0KPiB5b3VyIHBhdGNoIG9uIHRvcC4gIFRoYXQgd2F5IHdlIGNh biBkbyB0aGUgc3BlYXIxM3h4IGJ1ZyBmaXggYnkgaXRzZWxmDQo+ICh3aGljaCBzaG91bGQgYmUg YSBmYWlybHkgb2J2aW91c2x5LWNvcnJlY3QgcGF0Y2gpLCB0aGVuIHlvdXINCj4gaW50ZXJmYWNl IGNoYW5nZSAod2hpY2ggd291bGQgYmUgZXh0ZW5kZWQgdG8gdG91Y2ggYWxsIHRoZSBjYWxscw0K PiBpbmNsdWRpbmcgc3BlYXIxM3h4IGFuZCBiZSBzaW1pbGFybHkgb2J2aW91c2x5IGNvcnJlY3Qp LiAgSSB0aGluayB0aGF0DQo+IHdvdWxkIG1ha2UgaXQgYSBiaXQgZWFzaWVyIGZvciBmdXR1cmUg Y2hhbmdlbG9nIGFyY2hhZW9sb2dpc3RzLg0KDQpTaW5jZSBQcmF0eXVzaCBoYXMgbm90IHB1c2hl ZCB0aGUgZml4IHlldCwgbWF5YmUgSSBjYW4gcHJlcGFyZSBhIHYyIGFzIA0KYSBwYXRjaHNldCBv ZiAyIHBhdGNoZXMuLi4uDQoodG8gYmVuZWZpdCB0aGUgTGludXggYXJjaGVvbG9naXN0cyBhcyB5 b3Ugc2FpZCA6LSkgKQ0KDQo+IA0KPiBCam9ybg0K -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c index f9f468d..8b0e04b 100644 --- a/drivers/pci/host/pci-exynos.c +++ b/drivers/pci/host/pci-exynos.c @@ -454,7 +454,7 @@ static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size, int ret; exynos_pcie_sideband_dbi_r_mode(pp, true); - ret = dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where, size, val); + ret = dw_pcie_cfg_read(pp->dbi_base, where, size, val); exynos_pcie_sideband_dbi_r_mode(pp, false); return ret; } @@ -465,8 +465,7 @@ static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size, int ret; exynos_pcie_sideband_dbi_w_mode(pp, true); - ret = dw_pcie_cfg_write(pp->dbi_base + (where & ~0x3), - where, size, val); + ret = dw_pcie_cfg_write(pp->dbi_base, where, size, val); exynos_pcie_sideband_dbi_w_mode(pp, false); return ret; } diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c index f34892e..2b391f4 100644 --- a/drivers/pci/host/pci-keystone-dw.c +++ b/drivers/pci/host/pci-keystone-dw.c @@ -403,7 +403,7 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn); - return dw_pcie_cfg_read(addr + (where & ~0x3), where, size, val); + return dw_pcie_cfg_read(addr, where, size, val); } int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, @@ -415,7 +415,7 @@ int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn); - return dw_pcie_cfg_write(addr + (where & ~0x3), where, size, val); + return dw_pcie_cfg_write(addr, where, size, val); } /** diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 69486be..b53aa38 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -82,13 +82,20 @@ static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys) int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val) { + addr += (where & ~0x3); *val = readl(addr); - - if (size == 1) - *val = (*val >> (8 * (where & 3))) & 0xff; - else if (size == 2) - *val = (*val >> (8 * (where & 3))) & 0xffff; - else if (size != 4) + where &= 3; + + if (size == 1) { + *val = (*val >> (8 * where)) & 0xff; + } else if (size == 2) { + if (where & 1) + return PCIBIOS_BAD_REGISTER_NUMBER; + *val = (*val >> (8 * where)) & 0xffff; + } else if (size == 4) { + if (where & 3) + return PCIBIOS_BAD_REGISTER_NUMBER; + } else return PCIBIOS_BAD_REGISTER_NUMBER; return PCIBIOS_SUCCESSFUL; @@ -96,12 +103,18 @@ int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val) int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 val) { - if (size == 4) + addr += where; + + if (size == 4) { + if ((uintptr_t)addr & 3) + return PCIBIOS_BAD_REGISTER_NUMBER; writel(val, addr); - else if (size == 2) - writew(val, addr + (where & 2)); - else if (size == 1) - writeb(val, addr + (where & 3)); + } else if (size == 2) { + if ((uintptr_t)addr & 1) + return PCIBIOS_BAD_REGISTER_NUMBER; + writew(val, addr); + } else if (size == 1) + writeb(val, addr); else return PCIBIOS_BAD_REGISTER_NUMBER; @@ -132,8 +145,7 @@ static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size, if (pp->ops->rd_own_conf) ret = pp->ops->rd_own_conf(pp, where, size, val); else - ret = dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where, - size, val); + ret = dw_pcie_cfg_read(pp->dbi_base, where, size, val); return ret; } @@ -146,8 +158,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size, if (pp->ops->wr_own_conf) ret = pp->ops->wr_own_conf(pp, where, size, val); else - ret = dw_pcie_cfg_write(pp->dbi_base + (where & ~0x3), where, - size, val); + ret = dw_pcie_cfg_write(pp->dbi_base, where, size, val); return ret; } @@ -541,13 +552,12 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, u32 devfn, int where, int size, u32 *val) { int ret, type; - u32 address, busdev, cfg_size; + u32 busdev, cfg_size; u64 cpu_addr; void __iomem *va_cfg_base; busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | PCIE_ATU_FUNC(PCI_FUNC(devfn)); - address = where & ~0x3; if (bus->parent->number == pp->root_bus_nr) { type = PCIE_ATU_TYPE_CFG0; @@ -564,7 +574,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus, dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, type, cpu_addr, busdev, cfg_size); - ret = dw_pcie_cfg_read(va_cfg_base + address, where, size, val); + ret = dw_pcie_cfg_read(va_cfg_base, where, size, val); dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, PCIE_ATU_TYPE_IO, pp->io_mod_base, pp->io_bus_addr, pp->io_size); @@ -576,13 +586,12 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, u32 devfn, int where, int size, u32 val) { int ret, type; - u32 address, busdev, cfg_size; + u32 busdev, cfg_size; u64 cpu_addr; void __iomem *va_cfg_base; busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) | PCIE_ATU_FUNC(PCI_FUNC(devfn)); - address = where & ~0x3; if (bus->parent->number == pp->root_bus_nr) { type = PCIE_ATU_TYPE_CFG0; @@ -599,7 +608,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus, dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, type, cpu_addr, busdev, cfg_size); - ret = dw_pcie_cfg_write(va_cfg_base + address, where, size, val); + ret = dw_pcie_cfg_write(va_cfg_base, where, size, val); dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, PCIE_ATU_TYPE_IO, pp->io_mod_base, pp->io_bus_addr, pp->io_size);