| Message ID | 1460414707-19153-3-git-send-email-jchandra@broadcom.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 04/11/2016 03:45 PM, Jayachandran C wrote: > Add config option PCI_GENERIC_ECAM and file drivers/pci/ecam.c to > provide generic functions for accessing memory mapped PCI config space. > > The API is defined in drivers/pci/ecam.h and is written to replace the > API in drivers/pci/host/pci-host-common.h. The file defines a new > 'struct pci_config_window' to hold the information related to a PCI > config area and its mapping. This structure is expected to be used as > sysdata for controllers that have ECAM based mapping. > > Helper functions are provided to setup the mapping, free the mapping > and to implement the map_bus method in 'struct pci_ops' > > Signed-off-by: Jayachandran C <jchandra@broadcom.com> Tested-by: David Daney <david.daney@cavium.com> > --- > drivers/pci/Kconfig | 3 ++ > drivers/pci/Makefile | 2 + > drivers/pci/ecam.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++ > drivers/pci/ecam.h | 58 +++++++++++++++++++++++ I wonder if these files should go in drivers/pci/host ... I understand that you still have to use them from drivers/pci/acpi though. I will let others opine on this, but could you put the contents of ecam.h into include/linux/pci.h along with the pci_generic_config_*() declarations? If you did that, the contents of ecam.c could go into drivers/pci/access.c... > 4 files changed, 193 insertions(+) > create mode 100644 drivers/pci/ecam.c > create mode 100644 drivers/pci/ecam.h > [...] -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David, JC, On 04/11/2016 08:24 PM, David Daney wrote: > Tested-by: David Daney <david.daney@cavium.com> On ThunderX (please let me know the silicon pass specifics off-list)? I'm planning to give this series a test run also on some other ARMv8 hardware and will prod a few of the other vendors to do so. >> drivers/pci/ecam.c | 130 >> drivers/pci/ecam.h | 58 +++++++++++++++++++++++ > > I wonder if these files should go in drivers/pci/host ... I understand > that you still have to use them from drivers/pci/acpi though. > > I will let others opine on this, but could you put the contents of > ecam.h into include/linux/pci.h along with the pci_generic_config_*() > declarations? > > If you did that, the contents of ecam.c could go into > drivers/pci/access.c... Quoting Bjorn's original reply to the previous series: > Some of the code that moved to drivers/acpi/pci_mcfg.c is not > really ACPI-specific, and could potentially be used for non-ACPI > bridges that support ECAM. I'd like to see that sort of code > moved to a new file like drivers/pci/ecam.c. So my guess is that this is the reasoning behind JC's file layout. I'm curious what Lorenzo's take on things is currently. I assume this series is now to be the official coordinated version of this effort for upstream, following the advice of Bjorn previously, but I would like to know if everyone is behind this plan. I've (previously) requested a Linaro LEG meeting this week (part of our bootarch working group) to specifically discuss the status of PCI upstreaming in order to get the different vendors together to ensure every single one of them is tracking the correct latest effort and doing what is needed to test/aid, hence my ask. If this is now plan A, I'll make sure everyone is aligned behind it and start pinging people individually for testing. Jon.
On Tue, Apr 12, 2016 at 12:26:25AM -0400, Jon Masters wrote: [...] > Quoting Bjorn's original reply to the previous series: > > > Some of the code that moved to drivers/acpi/pci_mcfg.c is not > > really ACPI-specific, and could potentially be used for non-ACPI > > bridges that support ECAM. I'd like to see that sort of code > > moved to a new file like drivers/pci/ecam.c. > > So my guess is that this is the reasoning behind JC's file layout. > > I'm curious what Lorenzo's take on things is currently. I assume this > series is now to be the official coordinated version of this effort for > upstream, following the advice of Bjorn previously, but I would like to > know if everyone is behind this plan. I've (previously) requested a > Linaro LEG meeting this week (part of our bootarch working group) to > specifically discuss the status of PCI upstreaming in order to get the > different vendors together to ensure every single one of them is > tracking the correct latest effort and doing what is needed to test/aid, > hence my ask. If this is now plan A, I'll make sure everyone is aligned > behind it and start pinging people individually for testing. My take is that JC's aim is to get this four patch series reviewed and merged (which is *not* sufficient to get ACPI PCI to work fully on ARM64 - see cover letter - the remaining patches in his branch are not fixes, it is code that is required to get things to work, these 4 patches stand alone are not sufficient but I understand he wants to get them reviewed following feedback on the lists) so that we can make progress on ACPI PCI on ARM64. I will comment on the patches as soon as I have time to review them, I certainly would like to understand what we have to do with the rest of the code though (provided this series is good to go) see above. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/12/2016 12:44 PM, Lorenzo Pieralisi wrote: > On Tue, Apr 12, 2016 at 12:26:25AM -0400, Jon Masters wrote: > > [...] > >> Quoting Bjorn's original reply to the previous series: >> >>> Some of the code that moved to drivers/acpi/pci_mcfg.c is not >>> really ACPI-specific, and could potentially be used for non-ACPI >>> bridges that support ECAM. I'd like to see that sort of code >>> moved to a new file like drivers/pci/ecam.c. >> >> So my guess is that this is the reasoning behind JC's file layout. >> >> I'm curious what Lorenzo's take on things is currently. I assume this >> series is now to be the official coordinated version of this effort for >> upstream, following the advice of Bjorn previously, but I would like to >> know if everyone is behind this plan. I've (previously) requested a >> Linaro LEG meeting this week (part of our bootarch working group) to >> specifically discuss the status of PCI upstreaming in order to get the >> different vendors together to ensure every single one of them is >> tracking the correct latest effort and doing what is needed to test/aid, >> hence my ask. If this is now plan A, I'll make sure everyone is aligned >> behind it and start pinging people individually for testing. > > My take is that JC's aim is to get this four patch series reviewed and > merged Indeed, I see that's probably the goal, and why not :) > (which is *not* sufficient to get ACPI PCI to work fully on ARM64 > - see cover letter - the remaining patches in his branch are not > fixes, it is code that is required to get things to work, these 4 > patches stand alone are not sufficient but I understand he wants to get > them reviewed following feedback on the lists) so that we can make > progress on ACPI PCI on ARM64. Agreed. I went through the branch and the 11 patches there, reacquainted myself with what's what. So what we have now is 4 patches here plus a few others that in total replace v5 of your previous mmconfig patches in functionality. The question is what happens with the rest (of JC's branch let's say) - do they get sent out now too? > I will comment on the patches as soon as I have time to review > them, I certainly would like to understand what we have to do with the > rest of the code though (provided this series is good to go) see above. Right. That's my reason for asking. I'd like to know who is driving (I believe that to be Lorenzo) and what the path forward is, and whether we need to get additional support from anyone else. There's a multi-vendor meeting in the morning where I'm going to summarize the current state of these patches and I would like to know (soon) what the plan is so that we can get everyone on deck to help out at least with testing (most have tested the previous set, but we need public acks happening). Jon. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 14, 2016 at 01:55:32AM -0400, Jon Masters wrote: [...] > > I will comment on the patches as soon as I have time to review > > them, I certainly would like to understand what we have to do with the > > rest of the code though (provided this series is good to go) see above. > > Right. That's my reason for asking. I'd like to know who is driving (I > believe that to be Lorenzo) and what the path forward is, and whether we > need to get additional support from anyone else. There's a multi-vendor > meeting in the morning where I'm going to summarize the current state of > these patches and I would like to know (soon) what the plan is so that > we can get everyone on deck to help out at least with testing (most have > tested the previous set, but we need public acks happening). Testing always helps, this code needs reviewing from PCI and ACPI standpoints, what we can do is review this series and repost the whole thing when we agree the ECAM refactoring this series is implementing is the right way to go and it is a step in that direction, with no generic MCFG support in the kernel ACPI PCI on ARM64 can't be implemented so I would start from that. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 12, 2016 at 5:54 AM, David Daney <ddaney@caviumnetworks.com> wrote: > On 04/11/2016 03:45 PM, Jayachandran C wrote: >> >> Add config option PCI_GENERIC_ECAM and file drivers/pci/ecam.c to >> provide generic functions for accessing memory mapped PCI config space. >> >> The API is defined in drivers/pci/ecam.h and is written to replace the >> API in drivers/pci/host/pci-host-common.h. The file defines a new >> 'struct pci_config_window' to hold the information related to a PCI >> config area and its mapping. This structure is expected to be used as >> sysdata for controllers that have ECAM based mapping. >> >> Helper functions are provided to setup the mapping, free the mapping >> and to implement the map_bus method in 'struct pci_ops' >> >> Signed-off-by: Jayachandran C <jchandra@broadcom.com> > > Tested-by: David Daney <david.daney@cavium.com> I have updated the git tree (https://github.com/jchandra-brcm/linux/) with a branch arm64-acpi-pci-v3 . The branch has a new patch to use thunder ECAM ops in case of Cavium ThunderX platform when doing generic ACPI PCI initialization. I am hoping that the controllers that have "ECAM with quirks" can use this mechanism for sharing the quirks between OF and ACPI. If you have some time to review the patch and see it works for you, then I can post it with the v3 of this patchset. >> --- >> drivers/pci/Kconfig | 3 ++ >> drivers/pci/Makefile | 2 + >> drivers/pci/ecam.c | 130 >> +++++++++++++++++++++++++++++++++++++++++++++++++++ >> drivers/pci/ecam.h | 58 +++++++++++++++++++++++ > > > I wonder if these files should go in drivers/pci/host ... I understand that > you still have to use them from drivers/pci/acpi though. > > I will let others opine on this, but could you put the contents of ecam.h > into include/linux/pci.h along with the pci_generic_config_*() > declarations? > > If you did that, the contents of ecam.c could go into > drivers/pci/access.c... Earlier discussion seems to indicated that separate ecam.c/h was preferred. But I agree that it may be small enough to be merged. Thanks, JC. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" 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/Kconfig b/drivers/pci/Kconfig index 209292e..e930d62 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -83,6 +83,9 @@ config HT_IRQ config PCI_ATS bool +config PCI_GENERIC_ECAM + bool + config PCI_IOV bool "PCI IOV support" depends on PCI diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 2154092..810aec8 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -55,6 +55,8 @@ obj-$(CONFIG_PCI_SYSCALL) += syscall.o obj-$(CONFIG_PCI_STUB) += pci-stub.o +obj-$(CONFIG_PCI_GENERIC_ECAM) += ecam.o + obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o obj-$(CONFIG_OF) += of.o diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c new file mode 100644 index 00000000..798f0b7 --- /dev/null +++ b/drivers/pci/ecam.c @@ -0,0 +1,130 @@ +/* + * Copyright 2016 Broadcom + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation (the "GPL"). + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License version 2 (GPLv2) for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 (GPLv2) along with this source code. + */ + +#include <linux/device.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/pci.h> +#include <linux/slab.h> + +#include "ecam.h" + +/* + * On 64 bit systems, we do a single ioremap for the whole config space + * since we have enough virtual address range available. On 32 bit, do an + * ioremap per bus. + */ +static const bool per_bus_mapping = !config_enabled(CONFIG_64BIT); + +/* + * Create a PCI config space window + * - reserve mem region + * - alloc struct pci_config_window with space for all mappings + * - ioremap the config space + */ +struct pci_config_window *pci_generic_ecam_create(struct device *dev, + phys_addr_t addr, u8 bus_start, u8 bus_end, + struct pci_generic_ecam_ops *ops) +{ + struct pci_config_window *cfg; + unsigned int bus_shift, bus_range, bsz, mapsz; + int i, nidx; + + if (bus_end < bus_start) + return ERR_PTR(-EINVAL); + + bus_shift = ops->bus_shift; + bus_range = bus_end - bus_start + 1; + bsz = 1 << bus_shift; + nidx = per_bus_mapping ? bus_range : 1; + mapsz = per_bus_mapping ? bsz : bus_range * bsz; + cfg = kzalloc(sizeof(*cfg) + nidx * sizeof(cfg->win[0]), GFP_KERNEL); + if (!cfg) + return ERR_PTR(-ENOMEM); + + cfg->bus_start = bus_start; + cfg->bus_end = bus_end; + cfg->ops = ops; + + if (!request_mem_region(addr, bus_range * bsz, "Configuration Space")) + goto err_exit; + + /* cfgaddr has to be set after request_mem_region */ + cfg->cfgaddr = addr; + + for (i = 0; i < nidx; i++) { + cfg->win[i] = ioremap(addr + i * mapsz, mapsz); + if (!cfg->win[i]) + goto err_exit; + } + return cfg; + +err_exit: + pci_generic_ecam_free(cfg); + return ERR_PTR(-ENOMEM); +} + +/* + * Free a config space mapping + */ +void pci_generic_ecam_free(struct pci_config_window *cfg) +{ + unsigned int bus_range; + int i, nidx; + + bus_range = cfg->bus_end - cfg->bus_start + 1; + nidx = per_bus_mapping ? bus_range : 1; + for (i = 0; i < nidx; i++) + if (cfg->win[i]) + iounmap(cfg->win[i]); + if (cfg->cfgaddr) + release_mem_region(cfg->cfgaddr, + bus_range << cfg->ops->bus_shift); + kfree(cfg); +} + +/* + * Function to implement the pci_ops ->map_bus method + */ +void __iomem *pci_generic_ecam_map_bus(struct pci_bus *bus, unsigned int devfn, + int where) +{ + struct pci_config_window *cfg = bus->sysdata; + unsigned int devfn_shift = cfg->ops->bus_shift - 8; + unsigned int busn = bus->number; + void __iomem *base; + + if (busn < cfg->bus_start || busn > cfg->bus_end) + return NULL; + + busn -= cfg->bus_start; + if (per_bus_mapping) + base = cfg->win[busn]; + else + base = cfg->win[0] + (busn << cfg->ops->bus_shift); + return base + (devfn << devfn_shift) + where; +} + +/* default ECAM ops */ +struct pci_generic_ecam_ops pci_generic_ecam_default_ops = { + .bus_shift = 20, + .ops = { + .map_bus = pci_generic_ecam_map_bus, + .read = pci_generic_config_read, + .write = pci_generic_config_write, + } +}; diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h new file mode 100644 index 00000000..dda8c50 --- /dev/null +++ b/drivers/pci/ecam.h @@ -0,0 +1,58 @@ +/* + * Copyright 2016 Broadcom + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation (the "GPL"). + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License version 2 (GPLv2) for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 (GPLv2) along with this source code. + */ +#ifndef DRIVERS_PCI_ECAM_H +#define DRIVERS_PCI_ECAM_H + +#include <linux/kernel.h> +#include <linux/platform_device.h> + +/* + * struct to hold pci ops and bus shift of the config window + * for a PCI controller. + */ +struct pci_generic_ecam_ops { + unsigned int bus_shift; + struct pci_ops ops; +}; + +/* + * struct to hold the mappings of a config space window. This + * will be allocated with enough entries in win[] to hold all + * the mappings for the bus range. + */ +struct pci_config_window { + phys_addr_t cfgaddr; + u16 domain; + u8 bus_start; + u8 bus_end; + void *priv; + struct pci_generic_ecam_ops *ops; + void __iomem *win[0]; +}; + +/* create and free for pci_config_window */ +struct pci_config_window *pci_generic_ecam_create(struct device *dev, + phys_addr_t addr, u8 bus_start, u8 bus_end, + struct pci_generic_ecam_ops *ops); +void pci_generic_ecam_free(struct pci_config_window *cfg); + +/* map_bus when ->sysdata is an instance of pci_config_window */ +void __iomem *pci_generic_ecam_map_bus(struct pci_bus *bus, unsigned int devfn, + int where); +/* default ECAM ops, bus shift 20, generic read and write */ +extern struct pci_generic_ecam_ops pci_generic_ecam_default_ops; + +#endif
Add config option PCI_GENERIC_ECAM and file drivers/pci/ecam.c to provide generic functions for accessing memory mapped PCI config space. The API is defined in drivers/pci/ecam.h and is written to replace the API in drivers/pci/host/pci-host-common.h. The file defines a new 'struct pci_config_window' to hold the information related to a PCI config area and its mapping. This structure is expected to be used as sysdata for controllers that have ECAM based mapping. Helper functions are provided to setup the mapping, free the mapping and to implement the map_bus method in 'struct pci_ops' Signed-off-by: Jayachandran C <jchandra@broadcom.com> --- drivers/pci/Kconfig | 3 ++ drivers/pci/Makefile | 2 + drivers/pci/ecam.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++ drivers/pci/ecam.h | 58 +++++++++++++++++++++++ 4 files changed, 193 insertions(+) create mode 100644 drivers/pci/ecam.c create mode 100644 drivers/pci/ecam.h