On Sunday 04 January 2015 20:19:34 Rob Herring wrote:
> Many PCI controllers' configuration space accesses are memory mapped
> varying only in address calculation and access checks. There are 2 main
> access methods: a decoded address space such as ECAM or a single address
> and data register similar to x86. This implementation can support both
> cases as well as be used in cases that need additional pre or post access
> handling.
>
> A new pci_ops member map_bus is introduced which can do access checks and
> any necessary setup. It returns the address to use for the configuration
> space access. The access types supported are 32-bit only accesses or
> correct byte, word, or dword sized accesses.
>
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
I think this looks very nice, and I don't mind using it as-is, but I'd
like to put up some variations for discussions so we get the best
implementation -- we should try not to change it again soon if someone
comes up with a slightly better way later ;-)
> I've converted a few drivers already. I'll send patches for them after
> some feedback on this. Most already have some function similar to what is
> needed for map_bus, so the conversion is pretty simple. This certainly
> isn't a complete list of possible users. The diffstat so far looks like
> this:
>
> arch/arm/mach-cns3xxx/pcie.c | 46 +++-------------------
> arch/arm/mach-integrator/pci_v3.c | 61 +++---------------------------
> arch/arm/mach-ks8695/pci.c | 75 +++---------------------------------
> arch/arm/mach-sa1100/pci-nanoengine.c | 94 ++++-----------------------------------------
> arch/powerpc/platforms/powermac/pci.c | 206 +++++++++++++++++++--------------------------------------------------------------------------------
> arch/powerpc/sysdev/fsl_pci.c | 46 ++--------------------
> drivers/pci/host/pci-host-generic.c | 51 ++-----------------------
> drivers/pci/host/pci-rcar-gen2.c | 51 ++-----------------------
> drivers/pci/host/pci-tegra.c | 55 ++-------------------------
> drivers/pci/host/pci-xgene.c | 150 +++++-------------------------------------------------------------------
> drivers/pci/host/pcie-xilinx.c | 88 +++++-------------------------------------
> 11 files changed, 93 insertions(+), 830 deletions(-)
Awesome!
> +int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
> + int where, int size, u32 *val)
> +{
> + void __iomem *addr;
> +
> + addr = bus->ops->map_bus(bus, devfn, where);
> + if (!addr) {
> + *val = ~0;
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + }
> +
> + if (size == 1)
> + *val = readb(addr);
> + else if (size == 2)
> + *val = readw(addr);
> + else
> + *val = readl(addr);
> +
> + return PCIBIOS_SUCCESSFUL;
> +}
PCI host controller drivers can be loadable modules these days, so
the functions clearly need to be exported.
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 360a966..e7fd519 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -560,6 +560,7 @@ static inline int pcibios_err_to_errno(int err)
> /* Low-level architecture-dependent routines */
>
> 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);
> };
In various other contexts, we have recently discussed adding new callbacks
to struct pci_host_bridge, or an operations structure below it. I don't see
a strong reason for one place or the other, but maybe someone else does.
If we put it into pci_host_bridge_ops, the first argument would need to
be the pci_host_bridge pointer of course.
For the common map_bus implementations, it would also be nice to put them
into the same file as your new access functions, but then we need a common
location to store at least one __iomem pointer. I guess that place could
either be struct pci_host_bridge or struct pci_bus. In theory, struct pci_ops
would work too, but then we could no longer mark it 'const' in host bridge
drivers.
If we have a common set of map_bus functions, we can even export the
pci_ops structures from drivers/pci/access.c:
const struct pci_ops pci_generic_ecam_ops = {
.map_bus = ecam_map_bus,
.read = pci_generic_config_read,
.write = pci_generic_config_write,
};
EXPORT_SYMBOL_GPL(pci_generic_ecam_ops);
That could of course be done in a follow-up patch, it doesn't have to be
part of your patch, but it would be good to be prepared.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
