Return-path: Received: from server19320154104.serverpool.info ([193.201.54.104]:54739 "EHLO hauke-m.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753541Ab3ALOpO (ORCPT ); Sat, 12 Jan 2013 09:45:14 -0500 Message-ID: <50F176EF.7080705@hauke-m.de> (sfid-20130112_154519_028913_61ADCCDC) Date: Sat, 12 Jan 2013 15:45:03 +0100 From: Hauke Mehrtens MIME-Version: 1.0 To: Nathan Hintz CC: linville@tuxdriver.com, linux-wireless@vger.kernel.org Subject: Re: [PATCH 3/5] bcma: don't map/unmap a subset of the PCI config space References: <1357987577-20661-1-git-send-email-nlhintz@hotmail.com> <50F17583.2000309@hauke-m.de> In-Reply-To: <50F17583.2000309@hauke-m.de> Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: On 01/12/2013 03:38 PM, Hauke Mehrtens wrote: > On 01/12/2013 11:46 AM, Nathan Hintz wrote: >> For PCI config space access offsets < 256 for device '0', >> bcma_extpci_write_config performs an 'ioremap_nocache' on a 4 byte >> section of the PCI config space (an area that has already >> previously been mapped), and then subsequently unmaps that 4 byte >> section. This can't be a good thing for future read access from >> that now unmapped location. Modify the config space writes to use >> the existing access functions (similar to how it is done for the reads). >> >> Signed-off-by: Nathan Hintz Acked-by: Hauke Mehrtens >> --- >> drivers/bcma/driver_pci_host.c | 22 +++++++++++----------- >> 1 files changed, 11 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/bcma/driver_pci_host.c b/drivers/bcma/driver_pci_host.c >> index 187cc9f..e1381ba 100644 >> --- a/drivers/bcma/driver_pci_host.c >> +++ b/drivers/bcma/driver_pci_host.c >> @@ -149,7 +149,7 @@ static int bcma_extpci_write_config(struct bcma_drv_pci *pc, unsigned int dev, >> const void *buf, int len) >> { >> int err = -EINVAL; >> - u32 addr = 0, val = 0; >> + u32 addr, val; >> void __iomem *mmio = 0; >> u16 chipid = pc->core->bus->chipinfo.id; >> >> @@ -165,12 +165,10 @@ static int bcma_extpci_write_config(struct bcma_drv_pci *pc, unsigned int dev, >> * requires indirect access. >> */ >> if (off < PCI_CONFIG_SPACE_SIZE) { >> - addr = pc->core->addr + BCMA_CORE_PCI_PCICFG0; >> + addr = BCMA_CORE_PCI_PCICFG0; >> addr |= (func << 8); >> addr |= (off & 0xfc); >> - mmio = ioremap_nocache(addr, sizeof(val)); >> - if (!mmio) >> - goto out; >> + val = pcicore_read32(pc, addr); >> } > > off >= PCI_CONFIG_SPACE_SIZE is not handled here, but I think it should > be. This part of the function should use the same code as used in > bcma_extpci_read_config(). > > Your patch improves this, but I think there is an other flaw in the code. I just saw you do this in the next patch, ignore this comment. > >> } else { >> addr = bcma_get_cfgspace_addr(pc, dev, func, off); >> @@ -189,12 +187,10 @@ static int bcma_extpci_write_config(struct bcma_drv_pci *pc, unsigned int dev, >> >> switch (len) { >> case 1: >> - val = readl(mmio); >> val &= ~(0xFF << (8 * (off & 3))); >> val |= *((const u8 *)buf) << (8 * (off & 3)); >> break; >> case 2: >> - val = readl(mmio); >> val &= ~(0xFFFF << (8 * (off & 3))); >> val |= *((const u16 *)buf) << (8 * (off & 3)); >> break; >> @@ -202,13 +198,17 @@ static int bcma_extpci_write_config(struct bcma_drv_pci *pc, unsigned int dev, >> val = *((const u32 *)buf); >> break; >> } >> - if (dev == 0 && !addr) { >> + if (dev == 0) { >> /* accesses to config registers with offsets >= 256 >> * requires indirect access. >> */ >> - addr = (func << 12); >> - addr |= (off & 0x0FFF); >> - bcma_pcie_write_config(pc, addr, val); >> + if (off >= PCI_CONFIG_SPACE_SIZE) { >> + addr = (func << 12); >> + addr |= (off & 0x0FFF); >> + bcma_pcie_write_config(pc, addr, val); >> + } else { >> + pcicore_write32(pc, addr, val); >> + } >> } else { >> writel(val, mmio); >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >