Hi Bjorn, On Tuesday 17 October 2017 05:13 AM, Bjorn Helgaas wrote: > On Fri, Oct 13, 2017 at 06:09:11PM +0200, Niklas Cassel wrote: >> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxxx> >> --- >> .../devicetree/bindings/pci/axis,artpec6-pcie.txt | 3 +- >> drivers/pci/dwc/Kconfig | 41 +++-- >> drivers/pci/dwc/Makefile | 4 +- >> drivers/pci/dwc/pcie-artpec6.c | 202 ++++++++++++++++++++- >> 4 files changed, 233 insertions(+), 17 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt >> index 4e4aee4439ea..33eef7ae5a23 100644 >> --- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt >> +++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt >> @@ -4,7 +4,8 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP >> and thus inherits all the common properties defined in designware-pcie.txt. >> >> Required properties: >> -- compatible: "axis,artpec6-pcie", "snps,dw-pcie" >> +- compatible: "axis,artpec6-pcie", "snps,dw-pcie" for ARTPEC-6 in RC mode; >> + "axis,artpec6-pcie-ep", "snps,dw-pcie" for ARTPEC-6 in EP mode; >> - reg: base addresses and lengths of the PCIe controller (DBI), >> the PHY controller, and configuration address space. >> - reg-names: Must include the following entries: >> diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig >> index 22ec82fcdea2..e333283fb1ed 100644 >> --- a/drivers/pci/dwc/Kconfig >> +++ b/drivers/pci/dwc/Kconfig >> @@ -14,6 +14,36 @@ config PCIE_DW_EP >> depends on PCI_ENDPOINT >> select PCIE_DW >> >> +config PCIE_ARTPEC6 >> + bool "Axis ARTPEC-6 PCIe controller" >> + depends on (PCI && PCI_MSI_IRQ_DOMAIN) || PCI_ENDPOINT >> + depends on MACH_ARTPEC6 >> + help >> + Say Y here to enable PCIe controller support on Axis ARTPEC-6 >> + SoCs. This PCIe controller uses the DesignWare core. >> + >> +if PCIE_ARTPEC6 >> + >> +config PCIE_ARTPEC6_HOST >> + bool "Axis ARTPEC-6 Host Mode" >> + depends on PCI >> + depends on PCI_MSI_IRQ_DOMAIN >> + select PCIEPORTBUS >> + select PCIE_DW_HOST >> + help >> + Enables support for the PCIe controller in the ARTPEC-6 SoC to work in >> + host mode. >> + >> +config PCIE_ARTPEC6_EP >> + bool "Axis ARTPEC-6 Endpoint Mode" >> + depends on PCI_ENDPOINT >> + select PCIE_DW_EP >> + help >> + Enables support for the PCIe controller in the ARTPEC-6 SoC to work in >> + endpoint mode. >> + >> +endif >> + >> config PCI_DRA7XX >> bool "TI DRA7xx PCIe controller" >> depends on SOC_DRA7XX || COMPILE_TEST >> @@ -148,17 +178,6 @@ config PCIE_ARMADA_8K >> DesignWare hardware and therefore the driver re-uses the >> DesignWare core functions to implement the driver. >> >> -config PCIE_ARTPEC6 >> - bool "Axis ARTPEC-6 PCIe controller" >> - depends on PCI >> - depends on MACH_ARTPEC6 >> - depends on PCI_MSI_IRQ_DOMAIN >> - select PCIEPORTBUS >> - select PCIE_DW_HOST >> - help >> - Say Y here to enable PCIe controller support on Axis ARTPEC-6 >> - SoCs. This PCIe controller uses the DesignWare core. >> - >> config PCIE_KIRIN >> depends on OF && ARM64 >> bool "HiSilicon Kirin series SoCs PCIe controllers" > >> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile >> index c61be9738cce..ac98242b83a9 100644 >> --- a/drivers/pci/dwc/Makefile >> +++ b/drivers/pci/dwc/Makefile >> @@ -2,6 +2,9 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o >> obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o >> obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o >> obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o >> +ifneq ($(filter y,$(CONFIG_PCIE_ARTPEC6_HOST) $(CONFIG_PCIE_ARTPEC6_EP)),) >> + obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o >> +endif > > I see you're copying the DRA7XX style here, but I don't really > understand it. I guess the idea is to build pcie-artpec6.o if either > CONFIG_PCIE_ARTPEC6_HOST or CONFIG_PCIE_ARTPEC6_EP is set (or both). > > Is this really the simplest way to express that in Kconfig? Both the > "if PCIE_ARTPEC6" and this ifneq thing are complicated and relatively > unusual. > >> ifneq ($(filter y,$(CONFIG_PCI_DRA7XX_HOST) $(CONFIG_PCI_DRA7XX_EP)),) >> obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o >> endif > >> +static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) > > Looks funny to have this dw_*() function in pcie-artpec6.c. Intended? > Or is this something other endpoint drivers could/should share? yeah, dw_pcie_ep_reset_bar is already present in pcie-designware-ep.c which should be made as library function to be used by other drivers. (pci-dra7xx.c has also added something like above which should also be fixed). Thanks Kishon
