On Friday, May 27, 2016 6:31:28 PM CEST Wenrui Li wrote: > Hi, > > On 2016/5/27 15:13, Bharat Kumar Gogada wrote: > >>> > >>>> + > >>>> +static int rockchip_pcie_rd_other_conf(struct rockchip_pcie_port *pp, > >>>> + struct pci_bus *bus, u32 devfn, > >>>> + int where, int size, u32 *val) > >>>> +{ > >>>> + u32 busdev; > >>>> + > >>>> + busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn), > >>>> + PCI_FUNC(devfn), where); > >>>> + > >>>> + if (busdev & (size - 1)) { > >>>> + *val = 0; > >>>> + return PCIBIOS_BAD_REGISTER_NUMBER; > >>>> + } > >>>> + > >>>> + if (size == 4) { > >>>> + *val = readl(pp->reg_base + busdev); > >>>> + } else if (size == 2) { > >>>> + *val = readw(pp->reg_base + busdev); > >>>> + } else if (size == 1) { > >>>> + *val = readb(pp->reg_base + busdev); > >>>> + } else { > >>>> + *val = 0; > >>>> + return PCIBIOS_BAD_REGISTER_NUMBER; > >>>> + } > >>>> + return PCIBIOS_SUCCESSFUL; > >>>> +} > >>>> + > >>> > >>> This looks like the normal ECAM operations, you could just call those. > >> > >> I read ECAM reference code, I found it not support ioremap config space > >> for each bus individually on 64-bit systems. Our soc is 64-bit system, > >> and bus0 config space base address is 0xfda00000, bus1 base address is > >> 0xf8100000. So I think it is not normal ECAM operations, I do not know > >> if I have understood correctly? > >> > > Hi, > > > > I think Arnd was suggesting to use generic config read/write calls, pci_generic_config_read/pci_generic_config_write > > which does above functionality. > > Yeah, I seem the pci_generic_config_write use writew/writeb for byte and > word write. but our SOC not support byte and word write in RC config > spcace. So I redefine the the pci_ops.write Rihgt, that bug should be documented somewhere. You can also use the pci_generic_config_read32/pci_generic_config_write32 functions for this. I wonder if we should add a more general way of treating type 1 config space accesses differently, as a lot of host bridges have similar requirements, accessing the registers in a different place, different layout, or other constraints. The patch below would make that very easy to do. Suggestions for better naming welcome. Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx> diff --git a/drivers/pci/access.c b/drivers/pci/access.c index d11cdbb8fba3..2f7dcaa63e1d 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -34,9 +34,12 @@ int pci_bus_read_config_##size \ u32 data = 0; \ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \ raw_spin_lock_irqsave(&pci_lock, flags); \ - res = bus->ops->read(bus, devfn, pos, len, &data); \ + if (bus->number == 0 && bus->ops->read0) \ + res = bus->ops->read0(bus, devfn, pos, len, &data); \ + else \ + res = bus->ops->read(bus, devfn, pos, len, &data); \ *value = (type)data; \ - raw_spin_unlock_irqrestore(&pci_lock, flags); \ + raw_spin_unlock_irqrestore(&pci_lock, flags); \ return res; \ } @@ -48,8 +51,11 @@ int pci_bus_write_config_##size \ unsigned long flags; \ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \ raw_spin_lock_irqsave(&pci_lock, flags); \ - res = bus->ops->write(bus, devfn, pos, len, value); \ - raw_spin_unlock_irqrestore(&pci_lock, flags); \ + if (bus->number == 0 && bus->ops->write0) \ + res = bus->ops->write0(bus, devfn, pos, len, value); \ + else \ + res = bus->ops->write(bus, devfn, pos, len, value); \ + raw_spin_unlock_irqrestore(&pci_lock, flags); \ return res; \ } @@ -72,7 +78,11 @@ int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn, { void __iomem *addr; - addr = bus->ops->map_bus(bus, devfn, where); + if (bus->number == 0 && bus->ops->map_bus0) + addr = bus->ops->map_bus0(bus, devfn, where); + else + addr = bus->ops->map_bus(bus, devfn, where); + if (!addr) { *val = ~0; return PCIBIOS_DEVICE_NOT_FOUND; @@ -94,7 +104,10 @@ int pci_generic_config_write(struct pci_bus *bus, unsigned int devfn, { void __iomem *addr; - addr = bus->ops->map_bus(bus, devfn, where); + if (bus->number == 0 && bus->ops->map_bus0) + addr = bus->ops->map_bus0(bus, devfn, where); + else + addr = bus->ops->map_bus(bus, devfn, where); if (!addr) return PCIBIOS_DEVICE_NOT_FOUND; @@ -114,7 +127,10 @@ int pci_generic_config_read32(struct pci_bus *bus, unsigned int devfn, { void __iomem *addr; - addr = bus->ops->map_bus(bus, devfn, where & ~0x3); + if (bus->number == 0 && bus->ops->map_bus0) + addr = bus->ops->map_bus0(bus, devfn, where); + else + addr = bus->ops->map_bus(bus, devfn, where & ~0x3); if (!addr) { *val = ~0; return PCIBIOS_DEVICE_NOT_FOUND; @@ -135,7 +151,10 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, void __iomem *addr; u32 mask, tmp; - addr = bus->ops->map_bus(bus, devfn, where & ~0x3); + if (bus->number == 0 && bus->ops->map_bus0) + addr = bus->ops->map_bus0(bus, devfn, where); + else + addr = bus->ops->map_bus(bus, devfn, where & ~0x3); if (!addr) return PCIBIOS_DEVICE_NOT_FOUND; diff --git a/include/linux/pci.h b/include/linux/pci.h index df41c4645911..1caae3bd7115 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -580,6 +580,9 @@ struct pci_ops { void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where); int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val); int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val); + void __iomem *(*map_bus0)(struct pci_bus *bus, unsigned int devfn, int where); + int (*read0)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val); + int (*write0)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val); }; /* -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html