p71924506
/
linux_old1
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

PCI: mvebu: Use exact config access size; don't read/modify/write 

The idea that you can arbitarily read 32-bits from PCI configuration space,
modify a sub-field (like the command register) and write it back without
consequence is deeply flawed.

Status registers (such as the status register, PCIe device status register,
etc) contain status bits which are read, write-one-to-clear.

What this means is that reading 32-bits from the command register,
modifying the command register, and then writing it back has the effect of
clearing any status bits that were indicating at that time.  Same for the
PCIe device control register clearing bits in the PCIe device status
register.

Since the Armada chips support byte, 16-bit and 32-bit accesses to the
registers (unless otherwise stated) and the PCI configuration data register
does not specify otherwise, it seems logical that the chip can indeed
generate the proper configuration access cycles down to byte level.

Testing with an ASM1062 PCIe to SATA mini-PCIe card on Armada 388.  PCIe
capability at 0x80, DevCtl at 0x88, DevSta at 0x8a.

Before:
  /# setpci -s 1:0.0 0x88.l		- DevSta: CorrErr+
  00012810
  /# setpci -s 1:0.0 0x88.w=0x2810	- Write DevCtl only
  /# setpci -s 1:0.0 0x88.l		- CorrErr cleared - FAIL
  00002810

After:
  /# setpci -s 1:0.0 0x88.l		- DevSta: CorrErr+
  00012810
  /# setpci -s 1:0.0 0x88.w=0x2810	- check DevCtl only write
  /# setpci -s 1:0.0 0x88.l		- CorErr remains set
  00012810
  /# setpci -s 1:0.0 0x88.w=0x281f	- check DevCtl write works
  /# setpci -s 1:0.0 0x88.l		- devctl field updated
  0001281f
  /# setpci -s 1:0.0 0x8a.w=0xffff	- clear DevSta
  /# setpci -s 1:0.0 0x88.l		- CorrErr now cleared
  0000281f
  /# setpci -s 1:0.0 0x88.w=0x2810	- restore DevCtl
  /# setpci -s 1:0.0 0x88.l		- check
  00002810

[bhelgaas: changelog]
Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> (Armada XP GP)
Tested-by: Andrew Lunn <andrew@lunn.ch> (Kirkwood DIR665)
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
This commit is contained in:
Russell King 2015-09-23 18:17:32 +01:00 committed by Bjorn Helgaas
parent 58c19a140d
commit 79e3f6ce16
1 changed files with 26 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
drivers/pci/host/pci-mvebu.c
View File

@ -254,15 +254,22 @@ static int mvebu_pcie_hw_rd_conf(struct mvebu_pcie_port *port,
struct pci_bus *bus,
u32 devfn, int where, int size, u32 *val)
{
void __iomem *conf_data = port->base + PCIE_CONF_DATA_OFF;
mvebu_writel(port, PCIE_CONF_ADDR(bus->number, devfn, where),
PCIE_CONF_ADDR_OFF);
*val = mvebu_readl(port, PCIE_CONF_DATA_OFF);
if (size == 1)
*val = (*val >> (8 * (where & 3))) & 0xff;
else if (size == 2)
*val = (*val >> (8 * (where & 3))) & 0xffff;
switch (size) {
case 1:
*val = readb_relaxed(conf_data + (where & 3));
break;
case 2:
*val = readw_relaxed(conf_data + (where & 2));
break;
case 4:
*val = readl_relaxed(conf_data);
break;
}
return PCIBIOS_SUCCESSFUL;
}
@ -271,22 +278,24 @@ static int mvebu_pcie_hw_wr_conf(struct mvebu_pcie_port *port,
struct pci_bus *bus,
u32 devfn, int where, int size, u32 val)
{
u32 _val, shift = 8 * (where & 3);
void __iomem *conf_data = port->base + PCIE_CONF_DATA_OFF;
mvebu_writel(port, PCIE_CONF_ADDR(bus->number, devfn, where),
PCIE_CONF_ADDR_OFF);
_val = mvebu_readl(port, PCIE_CONF_DATA_OFF);
if (size == 4)
_val = val;
else if (size == 2)
_val = (_val & ~(0xffff << shift)) | ((val & 0xffff) << shift);
else if (size == 1)
_val = (_val & ~(0xff << shift)) | ((val & 0xff) << shift);
else
switch (size) {
case 1:
writeb(val, conf_data + (where & 3));
break;
case 2:
writew(val, conf_data + (where & 2));
break;
case 4:
writel(val, conf_data);
break;
default:
return PCIBIOS_BAD_REGISTER_NUMBER;
mvebu_writel(port, _val, PCIE_CONF_DATA_OFF);
}
return PCIBIOS_SUCCESSFUL;
}