| Message ID | 20191209092147.22901-1-kishon@ti.com |
|---|---|
| Headers | show |
| Series | Add PCIe support to TI's J721E SoC | expand |
On Mon, Dec 09, 2019 at 02:51:35PM +0530, Kishon Vijay Abraham I wrote: > commit bd22885aa188f135fd9 ("PCI: cadence: Refactor driver to use > as a core library") while refactoring the Cadence PCIe driver to be > used as library, removed pm_runtime_get_sync() from cdns_pcie_ep_setup() > and cdns_pcie_host_setup() but missed to remove the corresponding > pm_runtime_put_sync() in the error path. Fix it here. > > Fixes: commit bd22885aa188f135fd9 ("PCI: cadence: Refactor driver to use As this is a fix, a commit subject starting with PCI: cadence: Fix ... may be more obvious. I'd suggest you use the shorter form of this, i.e. Fixes: %h (\"%s\")) Fixes: bd22885aa188 ("PCI: cadence: Refactor driver to use as a core library") > as a core library") > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/controller/cadence/pcie-cadence-ep.c | 2 -- > drivers/pci/controller/cadence/pcie-cadence-host.c | 2 -- > 2 files changed, 4 deletions(-) > > diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c > index 1c173dad67d1..560f22b4d165 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c > @@ -473,7 +473,5 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep) > pci_epc_mem_exit(epc); > > err_init: > - pm_runtime_put_sync(dev); > - > return ret; > } > diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c > index 9b1c3966414b..ccf55e143e1d 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-host.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c > @@ -275,7 +275,5 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc) > pci_free_resource_list(&resources); > > err_init: > - pm_runtime_put_sync(dev); > - There is probably more you can do here for both -host and -ep: - Remove the possibly now unused <linux/pm_runtime.h> include - Remove the err_init label and return directly from source. Thanks, Andrew Murray > return ret; > } > -- > 2.17.1 >
Hi, On 16/12/19 7:37 pm, Andrew Murray wrote: > On Mon, Dec 09, 2019 at 02:51:37PM +0530, Kishon Vijay Abraham I wrote: >> Add support to use custom read and write accessors. Platforms that >> doesn't support half word or byte access or any other constraint > > s/doesn't/don't/ > >> while accessing registers can use this feature to populate custom >> read and write accessors. These custom accessors are used for both >> standard register access and configuration space register access. > > You can put the following sentence underneath a --- as it's not needed > in the commit message (but may be helpful to reviewers). > >> This is in preparation for adding PCIe support in TI's J721E SoC which >> uses Cadence PCIe core. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> drivers/pci/controller/cadence/pcie-cadence.h | 99 +++++++++++++++++-- >> 1 file changed, 90 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h >> index a2b28b912ca4..d0d91c69fa1d 100644 >> --- a/drivers/pci/controller/cadence/pcie-cadence.h >> +++ b/drivers/pci/controller/cadence/pcie-cadence.h >> @@ -223,6 +223,11 @@ enum cdns_pcie_msg_routing { >> MSG_ROUTING_GATHER, >> }; >> >> +struct cdns_pcie_ops { >> + u32 (*read)(void __iomem *addr, int size); >> + void (*write)(void __iomem *addr, int size, u32 value); >> +}; >> + >> /** >> * struct cdns_pcie - private data for Cadence PCIe controller drivers >> * @reg_base: IO mapped register base >> @@ -239,7 +244,7 @@ struct cdns_pcie { >> int phy_count; >> struct phy **phy; >> struct device_link **link; >> - const struct cdns_pcie_common_ops *ops; > > What was cdns_pcie_common_ops? It's not defined in the current tree is it? Yeah, it's spurious change that has got merged. > >> + const struct cdns_pcie_ops *ops; >> }; >> >> /** >> @@ -301,21 +306,47 @@ struct cdns_pcie_ep { >> /* Register access */ >> static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 value) >> { >> + void __iomem *addr = pcie->reg_base + reg; >> + >> + if (pcie->ops && pcie->ops->write) { >> + pcie->ops->write(addr, 0x1, value); >> + return; >> + } >> + >> writeb(value, pcie->reg_base + reg); > > Can you use 'addr' here instead of 'pcie->reg_base + reg'? (And similar for the > rest of them). Sure. Thanks Kishon
Hi Rob, On 19/12/19 5:38 AM, Rob Herring wrote: > On Mon, Dec 09, 2019 at 02:51:43PM +0530, Kishon Vijay Abraham I wrote: >> Add host mode dt-bindings for TI's J721E SoC. >> >> Cc: Rob Herring <robh+dt@kernel.org> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> .../bindings/pci/ti,j721e-pci-host.yaml | 161 ++++++++++++++++++ >> 1 file changed, 161 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> >> diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> new file mode 100644 >> index 000000000000..96184e1f419f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml >> @@ -0,0 +1,161 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/pci/ti,j721e-pci-host.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> + >> +title: TI J721E PCI Host (PCIe Wrapper) >> + >> +maintainers: >> + - Kishon Vijay Abraham I <kishon@ti.com> > > There's now a PCI bus schema. Reference it here: > > allOf: > - $ref: "/schemas/pci/pci-bus.yaml#" > >> + >> +properties: >> + compatible: >> + enum: >> + - ti,j721e-pcie-host > > Indentation. > >> + >> + reg: >> + maxItems: 4 >> + >> + reg-names: >> + items: >> + - const: intd_cfg >> + - const: user_cfg >> + - const: reg >> + - const: cfg >> + >> + ti,syscon-pcie-ctrl: >> + description: Phandle to the SYSCON entry required for configuring PCIe mode >> + and link speed. >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/phandle > > You can drop the 'allOf' here if there aren't more constraints. Do you mean I don't have to include phandle schema here? I don't seem to be able to include $ref without allOf. Thanks Kishon
TI's J721E SoC uses Cadence PCIe core to implement both RC mode and EP mode. The high level features are: *) Supports Legacy, MSI and MSI-X interrupt *) Supports upto GEN4 speed mode *) Supports SR-IOV *) Supports multiple physical function *) Ability to route all transactions via SMMU This patch series *) Add support in Cadence PCIe core to be used for TI's J721E SoC *) Add a driver for J721E PCIe wrapper This is a trimmed down series of the initial RFC series [1]. Changes from RFC [1]: *) Ability to route all transactions via SMMU is removed *) SR-IOV support is removed *) Miscellaneous improvement to endpoint core is removed All these will be sent as smaller series. I've also pushed the series along with device tree changes [2]. [1] -> https://lkml.org/lkml/2019/6/4/619 [2] -> https://github.com/kishon/linux-wip.git j7_pci_v1 Kishon Vijay Abraham I (13): PCI: cadence: Remove stray "pm_runtime_put_sync()" in error path linux/kernel.h: Add PTR_ALIGN_DOWN macro PCI: cadence: Add support to use custom read and write accessors PCI: cadence: Add support to start link and verify link status PCI: cadence: Add read and write accessors to perform only 32-bit accesses PCI: cadence: Allow pci_host_bridge to have custom pci_ops PCI: cadence: Add new *ops* for CPU addr fixup PCI: cadence: Use local management register to configure Vendor ID dt-bindings: PCI: Add host mode dt-bindings for TI's J721E SoC dt-bindings: PCI: Add EP mode dt-bindings for TI's J721E SoC PCI: j721e: Add TI J721E PCIe driver misc: pci_endpoint_test: Add J721E in pci_device_id table MAINTAINERS: Add Kishon Vijay Abraham I for TI J721E SoC PCIe .../bindings/pci/ti,j721e-pci-ep.yaml | 113 +++++ .../bindings/pci/ti,j721e-pci-host.yaml | 161 +++++++ MAINTAINERS | 3 +- drivers/misc/pci_endpoint_test.c | 9 + drivers/pci/controller/cadence/Kconfig | 23 + drivers/pci/controller/cadence/Makefile | 1 + drivers/pci/controller/cadence/pci-j721e.c | 430 ++++++++++++++++++ .../pci/controller/cadence/pcie-cadence-ep.c | 10 +- .../controller/cadence/pcie-cadence-host.c | 55 ++- drivers/pci/controller/cadence/pcie-cadence.c | 48 +- drivers/pci/controller/cadence/pcie-cadence.h | 133 +++++- include/linux/kernel.h | 1 + 12 files changed, 958 insertions(+), 29 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/ti,j721e-pci-ep.yaml create mode 100644 Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml create mode 100644 drivers/pci/controller/cadence/pci-j721e.c -- 2.17.1