[00/13] Add PCIe support to TI's J721E SoC

Message ID 20191209092147.22901-1-kishon@ti.com
Headers show
Series
  • Add PCIe support to TI's J721E SoC
Related show

Message

Kishon Vijay Abraham I Dec. 9, 2019, 9:21 a.m.
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

Comments

Andrew Murray Dec. 16, 2019, 1:45 p.m. | #1
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

>
Kishon Vijay Abraham I Dec. 19, 2019, 11:41 a.m. | #2
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
Kishon Vijay Abraham I Dec. 24, 2019, 8:06 a.m. | #3
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