PCI: Add Intel PCIe Driver and respective dt-binding yaml file
Subsystems (6)
v5 cover PCI: Add Intel PCIe Driver and respective dt-binding yaml file
Intel PCIe is Synopsys based controller utilizes the DesignWare framework for host initialization and Intel application specific register configurations.
Changes on v5:
Add Reviewed-by: Andrew Murray for device tree YAML schema patch. Address patchv4 review comments.
Sysfs patch work in progress, so not submitted in this patch revision. Add changes in artpec6 PCI driver to call dw helper function
for programming FTS.
Dilip Kota (3):
dt-bindings: PCI: intel: Add YAML schemas for the PCIe RC controller
dwc: PCI: intel: PCIe RC controller driver
PCI: artpec6: Configure FTS with dwc helper function
.../devicetree/bindings/pci/intel-gw-pcie.yaml | 138 ++++++ drivers/pci/controller/dwc/Kconfig | 10 + drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-artpec6.c | 8 +- drivers/pci/controller/dwc/pcie-designware.c | 57 +++ drivers/pci/controller/dwc/pcie-designware.h | 12 + drivers/pci/controller/dwc/pcie-intel-gw.c | 538 ++++++++++++++++ include/uapi/linux/pci_regs.h | 1 + 8 files changed, 758 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
Signature · 1 line
2.11.0
PATCH v5 1/3 dt-bindings: PCI: intel: Add YAML schemas for the PCIe RC controller
Add YAML schemas for PCIe RC controller on Intel Gateway SoCs which is Synopsys DesignWare based PCIe core.
---
Changes on v5:
Add Reviewed-by Andrew Murray.
Add possible values and default value for max-link-speed. Remove $ref and add maximum and default for reset-assert-ms. Set true flag for linux,pci-domain.
Define maxItems for ranges and clock.
Define maximum for num-lanes.
Update required list:
Add #address-cells, #size-cells, #interrupt-cells. Remove num-lanes and linux,pci-domain.
Add required header files in example.
Remove status entry in example.
changes on v4:
Add "snps,dw-pcie" compatible.
Rename phy-names property value to pcie. And maximum and minimum values to num-lanes. Add ref for reset-assert-ms entry and update the
description for easy understanding.
Remove PCIe core interrupt entry.
changes on v3:
Add the appropriate License-Identifier
Rename intel,rst-interval to 'reset-assert-us'
Add additionalProperties: false
Rename phy-names to 'pciephy'
Remove the dtsi node split of SoC and board in the example Add #interrupt-cells = <1>; or else interrupt parsing will fail Name yaml file with compatible name
.../devicetree/bindings/pci/intel-gw-pcie.yaml | 138 +++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml
Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml new file +138 -0
@@ -0,0 +1,138 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/intel-gw-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: PCIe RC controller on Intel Gateway SoCs + +maintainers: + - Dilip Kota <[email protected]> + +properties: + compatible: + items: + - const: intel,lgm-pcie + - const: snps,dw-pcie + + device_type: + const: pci + + "#address-cells": + const: 3 + + "#size-cells": + const: 2 + + reg: + items: + - description: Controller control and status registers. + - description: PCIe configuration registers. + - description: Controller application registers. + + reg-names: + items: + - const: dbi + - const: config + - const: app + + ranges: + maxItems: 1 + + resets: + maxItems: 1 + + clocks: + maxItems: 1 + + phys: + maxItems: 1 + + phy-names: + const: pcie + + reset-gpios: + maxItems: 1 + + linux,pci-domain: true + + num-lanes: + maximum: 2 + description: Number of lanes to use for this port. + + '#interrupt-cells': + const: 1 + + interrupt-map-mask: + description: Standard PCI IRQ mapping properties. + + interrupt-map: + description: Standard PCI IRQ mapping properties. + + max-link-speed: + description: Specify PCI Gen for link capability. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: [ 1, 2, 3, 4 ] + - default: 1 + + bus-range: + description: Range of bus numbers associated with this controller. + + reset-assert-ms: + description: | + Delay after asserting reset to the PCIe device. + maximum: 500 + default: 100 + +required: + - compatible + - device_type + - "#address-cells" + - "#size-cells" + - reg + - reg-names + - ranges + - resets + - clocks + - phys + - phy-names + - reset-gpios + - '#interrupt-cells' + - interrupt-map + - interrupt-map-mask + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/clock/intel,lgm-clk.h> + pcie10: pcie@d0e00000 { + compatible = "intel,lgm-pcie", "snps,dw-pcie"; + device_type = "pci"; + #address-cells = <3>; + #size-cells = <2>; + reg = <0xd0e00000 0x1000>, + <0xd2000000 0x800000>, + <0xd0a41000 0x1000>; + reg-names = "dbi", "config", "app"; + linux,pci-domain = <0>; + max-link-speed = <4>; + bus-range = <0x00 0x08>; + interrupt-parent = <&ioapic1>; + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0x7>; + interrupt-map = <0 0 0 1 &ioapic1 27 1>, + <0 0 0 2 &ioapic1 28 1>, + <0 0 0 3 &ioapic1 29 1>, + <0 0 0 4 &ioapic1 30 1>; + ranges = <0x02000000 0 0xd4000000 0xd4000000 0 0x04000000>; + resets = <&rcu0 0x50 0>; + clocks = <&cgu0 LGM_GCLK_PCIE10>; + phys = <&cb0phy0>; + phy-names = "pcie"; + reset-assert-ms = <500>; + reset-gpios = <&gpio0 3 GPIO_ACTIVE_LOW>; + num-lanes = <2>; + };
Signature · 1 line
2.11.0
Re: [PATCH v5 1/3] dt-bindings: PCI: intel: Add YAML schemas for the PCIe RC controller
Quoted 36 lines · Dilip Kota
Add YAML schemas for PCIe RC controller on Intel Gateway SoCs which is Synopsys DesignWare based PCIe core.
Signed-off-by Dilip Kota <[email protected]>Reviewed-by Andrew Murray <[email protected]>---
Changes on v5:
Add Reviewed-by Andrew Murray.
Add possible values and default value for max-link-speed. Remove $ref and add maximum and default for reset-assert-ms. Set true flag for linux,pci-domain.
Define maxItems for ranges and clock.
Define maximum for num-lanes.
Update required list:
Add #address-cells, #size-cells, #interrupt-cells. Remove num-lanes and linux,pci-domain.
Add required header files in example.
Remove status entry in example.changes on v4:
Add "snps,dw-pcie" compatible.
Rename phy-names property value to pcie. And maximum and minimum values to num-lanes. Add ref for reset-assert-ms entry and update the
description for easy understanding.
Remove PCIe core interrupt entry.changes on v3:
Add the appropriate License-Identifier
Rename intel,rst-interval to 'reset-assert-us'
Add additionalProperties: false
Rename phy-names to 'pciephy'
Remove the dtsi node split of SoC and board in the example Add #interrupt-cells = <1>; or else interrupt parsing will fail Name yaml file with compatible name
Quoted 3 lines
.../devicetree/bindings/pci/intel-gw-pcie.yaml | 138 +++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml
I'm working on a common PCI schema which will shrink this, but in the meantime:
Re: [PATCH v5 1/3] dt-bindings: PCI: intel: Add YAML schemas for the PCIe RC controller
Quoted 37 lines · Rob Herring
Quoted 36 lines · Dilip Kota
Add YAML schemas for PCIe RC controller on Intel Gateway SoCs which is Synopsys DesignWare based PCIe core.
Signed-off-by Dilip Kota <[email protected]>Reviewed-by Andrew Murray <[email protected]>---
Changes on v5:
Add Reviewed-by Andrew Murray.
Add possible values and default value for max-link-speed. Remove $ref and add maximum and default for reset-assert-ms. Set true flag for linux,pci-domain.
Define maxItems for ranges and clock.
Define maximum for num-lanes.
Update required list:
Add #address-cells, #size-cells, #interrupt-cells. Remove num-lanes and linux,pci-domain.
Add required header files in example.
Remove status entry in example.changes on v4:
Add "snps,dw-pcie" compatible.
Rename phy-names property value to pcie. And maximum and minimum values to num-lanes. Add ref for reset-assert-ms entry and update the
description for easy understanding.
Remove PCIe core interrupt entry.changes on v3:
Add the appropriate License-Identifier
Rename intel,rst-interval to 'reset-assert-us'
Add additionalProperties: false
Rename phy-names to 'pciephy'
Remove the dtsi node split of SoC and board in the example Add #interrupt-cells = <1>; or else interrupt parsing will fail Name yaml file with compatible name
Quoted 7 lines
Quoted 3 lines
.../devicetree/bindings/pci/intel-gw-pcie.yaml | 138 +++++++++++++++++++++ 1 file changed, 138 insertions(+)create mode 100644
Documentation/devicetree/bindings/pci/intel-gw-pcie.yamlI'm working on a common PCI schema which will shrink this, but in the meantime:
Reviewed-by Rob Herring <[email protected]>
Sure, Thanks for reviewing the patch.
Regards,
Dilip
PATCH v5 2/3 dwc: PCI: intel: PCIe RC controller driver
In reply to Dilip KotaAdd support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.
Intel PCIe driver requires Upconfigure support, fast training sequence and link speed configuration. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it in pci_regs.h.
---
Changes on v5:
Use new, old variables in pcie_update_bits()
Correct naming:
s/Designware/DesignWare
s/pcie/PCIe
s/pci/PCI
s/Hw/HW
Upconfig to Upconfigure
Remove extra lines after the intel_pcie_max_speed_setup() def. Use pcie_link_speed[] and remove enum for pcie gen. Remove dw_pcie_link_speed_change() def.
Remove "linux,pci-domain" parsing.
Remove 'id' variable in intel_pcie_port structure.
Remove extra spaces for lpp->link_gen =
Correct the offset of PCI_EXP_LNKCTL2_HASD to 0x0020. Remove programming of RCB and CCC bits in PCI_EXP_LCTL reg. Remove programming Slot clock cfg in PCI_EXP_LSTS reg. Update the comments at num_viewport setting.
Update the description in Kconfig.
Get PCIe cap offset from the registers using dw_pcie_find_capability() Define pcie_cap_ofst var in intel_pcie_port struct to store the same. Remove the PCIE_CAP_OFST macro.
Move intel_pcie_max_speed_setup() function to DesignWare framework,
defined as dw_pcie_link_set_max_speed()
Set EXPORT_SYMBOL_GPL for the newer functions defined in
pcie-designware.c
Changes on v4:
Rename the driver naming and description to
"PCIe RC controller on Intel Gateway SoCs".
Use PCIe core register macros defined in pci_regs.h
and remove respective local definitions.
Remove PCIe core interrupt handling.
Move PCIe link control speed change, upconfig and FTS. configuration functions to DesignWare framework.
Use of_pci_get_max_link_speed().
Mark dependency on X86 and COMPILE_TEST in Kconfig.
Remove lanes and add n_fts variables in intel_pcie_port structure. Rename rst_interval variable to rst_intrvl in intel_pcie_port structure. Remove intel_pcie_mem_iatu() as it is already perfomed in dw_setup_rc() Move sysfs attributes specific code to separate patch. Remove redundant error handling.
Reduce LoCs by doing variable initializations while declaration itself. Add extra line after closing parenthesis.
Move intel_pcie_ep_rst_init() out of get_resources()
changes on v3:
Rename PCIe app logic registers with PCIE_APP prefix. PCIE_IOP_CTRL configuration is not required. Remove respective code. Remove wrapper functions for clk enable/disable APIs.
Use platform_get_resource_byname() instead of
devm_platform_ioremap_resource() to be similar with DWC framework.
Rename phy name to "pciephy".
Modify debug message in msi_init() callback to be more specific. Remove map_irq() callback.
Enable the INTx interrupts at the time of PCIe initialization. Reduce memory fragmentation by using variable "struct dw_pcie pci"
instead of allocating memory.
Reduce the delay to 100us during enpoint initialization
intel_pcie_ep_rst_init().
Call dw_pcie_host_deinit() during remove() instead of directly
calling PCIe core APIs.
Rename "intel,rst-interval" to "reset-assert-ms".
Remove unused APP logic Interrupt bit macro definitions. Use dwc framework's dw_pcie_setup_rc() for PCIe host specific
configuration instead of redefining the same functionality in the driver.
Move the whole DT parsing specific code to intel_pcie_get_resources()
drivers/pci/controller/dwc/Kconfig | 10 + drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-designware.c | 57 +++ drivers/pci/controller/dwc/pcie-designware.h | 12 + drivers/pci/controller/dwc/pcie-intel-gw.c | 538 +++++++++++++++++++++++++++ include/uapi/linux/pci_regs.h | 1 + 6 files changed, 619 insertions(+) create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
drivers/pci/controller/dwc/Kconfig +10 -0
@@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP order to enable device-specific features PCI_DW_PLAT_EP must be selected. +config PCIE_INTEL_GW + bool "Intel Gateway PCIe host controller support" + depends on OF && (X86 || COMPILE_TEST) + select PCIE_DW_HOST + help + Say 'Y' here to enable PCIe Host controller support on Intel + Gateway SoCs. + The PCIe controller uses the DesignWare core plus Intel-specific + hardware wrappers. + config PCI_EXYNOS bool "Samsung Exynos PCIe controller" depends on SOC_EXYNOS5440 || COMPILE_TEST
drivers/pci/controller/dwc/Makefile +1 -0
@@ -3,6 +3,7 @@ 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 +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
drivers/pci/controller/dwc/pcie-designware.c +57 -0
69 lines not on this page · raw message
drivers/pci/controller/dwc/pcie-designware.h +12 -0
@@ -30,7 +30,12 @@#define LINK_WAIT_IATU 9/* Synopsys-specific PCIe configuration registers */ +#define PCIE_PORT_AFR 0x70C +#define PORT_AFR_N_FTS_MASK GENMASK(15, 8) +#define PORT_AFR_CC_N_FTS_MASK GENMASK(23, 16) +#define PCIE_PORT_LINK_CONTROL 0x710 +#define PORT_LINK_DLL_LINK_EN BIT(5)#define PORT_LINK_MODE_MASK GENMASK(21, 16)#define PORT_LINK_MODE (n) FIELD_PREP(PORT_LINK_MODE_MASK, n)#define PORT_LINK_MODE_1_LANES PORT_LINK_MODE(0x1)
@@ -46,6 +51,7 @@#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING BIT(29)#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C +#define PORT_LOGIC_N_FTS GENMASK(7, 0)#define PORT_LOGIC_SPEED_CHANGE BIT(17)#define PORT_LOGIC_LINK_WIDTH_MASK GENMASK(12, 8)#define PORT_LOGIC_LINK_WIDTH (n) FIELD_PREP(PORT_LOGIC_LINK_WIDTH_MASK, n)
@@ -60,6 +66,9 @@#define PCIE_MSI_INTR0_MASK 0x82C#define PCIE_MSI_INTR0_STATUS 0x830 +#define PCIE_PORT_MULTI_LANE_CTRL 0x8C0 +#define PORT_MLTI_UPCFG_SUPPORT BIT(7) +#define PCIE_ATU_VIEWPORT 0x900#define PCIE_ATU_REGION_INBOUND BIT(31)#define PCIE_ATU_REGION_OUTBOUND 0
@@ -273,6 +282,9 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);u32 dw_pcie_read_atu (struct dw_pcie *pci,u32 reg,size_t size);void dw_pcie_write_atu (struct dw_pcie *pci,u32 reg,size_t size,u32 val);int dw_pcie_link_up (struct dw_pcie *pci); +void dw_pcie_upconfig_setup (struct dw_pcie *pci); +void dw_pcie_link_set_max_speed (struct dw_pcie *pci,u32 link_gen); +void dw_pcie_link_set_n_fts (struct dw_pcie *pci,u32 n_fts);int dw_pcie_wait_for_link (struct dw_pcie *pci); void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type, u64 cpu_addr, u64 pci_addr,
drivers/pci/controller/dwc/pcie-intel-gw.c new file +538 -0
538 lines not on this page · raw message
include/uapi/linux/pci_regs.h +1 -0
@@ -673,6 +673,7 @@#define PCI_EXP_LNKCTL2_TLS_8_0GT 0x0003/* Supported Speed 8GT/s */ #define PCI_EXP_LNKCTL2_TLS_16_0GT 0x0004/* Supported Speed 16GT/s */ #define PCI_EXP_LNKCTL2_TLS_32_0GT 0x0005/* Supported Speed 32GT/s */ +#define PCI_EXP_LNKCTL2_HASD 0x0020/* HW Autonomous Speed Disable */ #define PCI_EXP_LNKSTA2 50/* Link Status 2 */ #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 52/* v2 endpoints with link end here */ #define PCI_EXP_SLTCAP2 52/* Slot Capabilities 2 */
Signature · 1 line
2.11.0
Re: [PATCH v5 2/3] dwc: PCI: intel: PCIe RC controller driver
Quoted 8 lines · Dilip Kota
Add support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.
Intel PCIe driver requires Upconfigure support, fast training sequence and link speed configuration. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it inpci_regs.h.
My comments below, though I may miss the discussion and comment on the settled things.
drivers/pci/controller/dwc/Kconfig config PCIE_DW_PLAT_EP · line 93
Quoted 9 lines
drivers/pci/controller/dwc/Kconfig +9
-0+config PCIE_INTEL_GW + bool "Intel Gateway PCIe host controller support" + depends on OF && (X86 || COMPILE_TEST) + select PCIE_DW_HOST + help + Say 'Y' here to enable PCIe Host controller support on Intel + Gateway SoCs. + The PCIe controller uses the DesignWare core plus Intel-specific + hardware wrappers.
Above has indentation issues.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 485
Quoted 7 lines
drivers/pci/controller/dwc/pcie-designware.c +7
-0+void dw_pcie_upconfig_setup (struct dw_pcie *pci) +{ +u32 val; + + val =dw_pcie_readl_dbi (pci,PCIE_PORT_MULTI_LANE_CTRL ); +dw_pcie_writel_dbi (pci,PCIE_PORT_MULTI_LANE_CTRL , + val |PORT_MLTI_UPCFG_SUPPORT );
Why not to use similar pattern as below?
val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL); val |= PORT_MLTI_UPCFG_SUPPORT; dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
Quoted 5 lines
drivers/pci/controller/dwc/pcie-designware.c +4
-0+} +void dw_pcie_link_set_max_speed (struct dw_pcie *pci,u32 link_gen) +{ +u32 reg, val;
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 500
Quoted 10 lines
drivers/pci/controller/dwc/pcie-designware.c +9
-0+u8 offset =dw_pcie_find_capability (pci,PCI_CAP_ID_EXP ); + + reg =dw_pcie_readl_dbi (pci, offset +PCI_EXP_LNKCTL2 ); + reg &= ~PCI_EXP_LNKCTL2_TLS ; + +switch (pcie_link_speed[link_gen]) { +case PCIE_SPEED_2_5GT : + reg |=PCI_EXP_LNKCTL2_TLS_2_5GT ; +break ;
Is this a style or indentation issue?
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 503
Quoted 3 lines
drivers/pci/controller/dwc/pcie-designware.c +3
-0+case PCIE_SPEED_5_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_5_0GT ; +break ;
Ditto.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 506
Quoted 4 lines
drivers/pci/controller/dwc/pcie-designware.c +3
-0+case PCIE_SPEED_8_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_8_0GT ; +break ;
Ditto.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 509
Quoted 4 lines
drivers/pci/controller/dwc/pcie-designware.c +3
-0+case PCIE_SPEED_16_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_16_0GT ; +break ;
Ditto.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 511
Quoted 3 lines
drivers/pci/controller/dwc/pcie-designware.c +2
-0+default : +/* Use hardware capability */
Ditto.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 516
Quoted 6 lines
drivers/pci/controller/dwc/pcie-designware.c +5
-0+ val =dw_pcie_readl_dbi (pci, offset +PCI_EXP_LNKCAP ); + val =FIELD_GET (PCI_EXP_LNKCAP_SLS , val); + reg &= ~PCI_EXP_LNKCTL2_HASD ; + reg |=FIELD_PREP (PCI_EXP_LNKCTL2_TLS , val); +break ;
Ditto.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 529
Quoted 13 lines
drivers/pci/controller/dwc/pcie-designware.c +11
-0+ } + +dw_pcie_writel_dbi (pci, offset +PCI_EXP_LNKCTL2 , reg); +} +void dw_pcie_link_set_n_fts (struct dw_pcie *pci,u32 n_fts) +{ +u32 val; + + val =dw_pcie_readl_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL ); + val &= ~PORT_LOGIC_N_FTS ; + val |= n_fts;
What if somebody supplies bits outside of the mask? I guess you need to apply proper masks to both values.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci)
Quoted 4 lines
drivers/pci/controller/dwc/pcie-designware.c +3
-0+dw_pcie_writel_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL , val); +} +#define PORT_LOGIC_N_FTS GENMASK(7, 0)
Shouldn't you use _MASK suffix here?
Quoted 1 line
+#define
BUS_IATU_OFFSSZ_256M
Perhaps less cryptic name?
Quoted 1 line
+#define
RST_INTRVL_DFT_MS100
Less cryptic name would be
RESET_INTERVAL_MS
drivers/pci/controller/dwc/pcie-intel-gw.c line 90
Quoted 8 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +7
-0+static void pcie_update_bits (void __iomem *base,u32 mask,u32 val,u32 ofs) +{ +u32 old, new; + + old =readl (base + ofs); + new = old & ~mask; + new |= val & mask;
Standard pattern
new = (old & ~mask) | (val & mask);
And actually you may re-use 'val' variable and get rid of 'new' one.
drivers/pci/controller/dwc/pcie-intel-gw.c line 100
Quoted 11 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +8
-0+ +if (new != old) +writel (new, base + ofs); +} +static int intel_pcie_get_resources (struct platform_device *pdev) +{ + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"dbi" ); +
No need to have this blank line.
drivers/pci/controller/dwc/pcie-intel-gw.c line 299
Quoted 7 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +5
-0+ pci->dbi_base =devm_ioremap_resource (dev, res); +if (IS_ERR (pci->dbi_base)) +return PTR_ERR (pci->dbi_base); + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"app" ); +
Ditto.
drivers/pci/controller/dwc/pcie-intel-gw.c line 334
Quoted 11 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +8
-0+ lpp->app_base =devm_ioremap_resource (dev, res); +if (IS_ERR (lpp->app_base)) +return PTR_ERR (lpp->app_base); +return 0 ; +} +/* Read PMC status and wait for falling into L2 link state */ + ret = readl_poll_timeout(lpp->app_base + PCIE_APP_PMC, value, + (value & PCIE_APP_PMC_IN_L2), 20,
Too many parentheses.
drivers/pci/controller/dwc/pcie-intel-gw.c line 374
Quoted 6 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +5
-0+ jiffies_to_usecs(5 * HZ)); +if (!lpp->pcie_cap_ofst) { + lpp->pcie_cap_ofst =dw_pcie_find_capability (&lpp->pci, +PCI_CAP_ID_EXP ); + }
Wouldn't be slightly better to have something like
ret = dw_pcie_find_capability(&lpp->pci, PCI_CAP_ID_EXP); if (ret >= 0 && !lpp->pcie_cap_ofst) lpp->pcie_cap_ofst = ret;
?
(It can be expanded to print error / warning messages if needed)
drivers/pci/controller/dwc/pcie-intel-gw.c line 484
Quoted 4 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +3
-0+return ret; +} +platform_set_drvdata (pdev, lpp);
I think it makes sense to setup at the end of the function (before dev_info() call).
drivers/pci/controller/dwc/pcie-intel-gw.c line 497
Quoted 1 line
drivers/pci/controller/dwc/pcie-intel-gw.c new file +1
-0+ data =device_get_match_data (dev);
Perhaps
if (!data) return -ENODEV; // -EINVAL?
drivers/pci/controller/dwc/pcie-intel-gw.c line 513
Quoted 5 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +5
-0+/* +* Intel PCIe doesn't configure IO region, so set viewport +* to not to perform IO region access. +*/ + pci->num_viewport = data->num_viewport;
Missed blank line?
Quoted 1 line
drivers/pci/controller/dwc/pcie-intel-gw.c new file +1
-0+dev_info (dev,"Intel PCIe Root Complex Port init done\n" );
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v5 2/3] dwc: PCI: intel: PCIe RC controller driver
Quoted 12 lines · Andy Shevchenko
Quoted 8 lines · Dilip Kota
Add support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.
Intel PCIe driver requires Upconfigure support, fast training sequence and link speed configuration. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it inpci_regs.h.My comments below, though I may miss the discussion and comment on the settled things.
Quoted 10 lines
drivers/pci/controller/dwc/Kconfig config PCIE_DW_PLAT_EP · line 93
Quoted 9 lines
drivers/pci/controller/dwc/Kconfig +9
-0+config PCIE_INTEL_GW + bool "Intel Gateway PCIe host controller support" + depends on OF && (X86 || COMPILE_TEST) + select PCIE_DW_HOST + help + Say 'Y' here to enable PCIe Host controller support on Intel + Gateway SoCs. + The PCIe controller uses the DesignWare core plus Intel-specific + hardware wrappers.Above has indentation issues.
Typo error, i will fix it.
Thanks for pointing.
Quoted 10 lines
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 485
Quoted 8 lines
drivers/pci/controller/dwc/pcie-designware.c +7
-0+void dw_pcie_upconfig_setup (struct dw_pcie *pci) +{ +u32 val; + + val =dw_pcie_readl_dbi (pci,PCIE_PORT_MULTI_LANE_CTRL ); +dw_pcie_writel_dbi (pci,PCIE_PORT_MULTI_LANE_CTRL , + val |PORT_MLTI_UPCFG_SUPPORT );Why not to use similar pattern as below?
Quoted 3 lines
val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL); val |= PORT_MLTI_UPCFG_SUPPORT; dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
Ok, i will update it.
Quoted 6 lines
Quoted 6 lines
drivers/pci/controller/dwc/pcie-designware.c +6
-0+} +void dw_pcie_link_set_max_speed (struct dw_pcie *pci,u32 link_gen) +{ +u32 reg, val; +u8 offset =dw_pcie_find_capability (pci,PCI_CAP_ID_EXP ); +
Quoted 8 lines
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 500
Quoted 7 lines
drivers/pci/controller/dwc/pcie-designware.c +7
-0+ reg =dw_pcie_readl_dbi (pci, offset +PCI_EXP_LNKCTL2 ); + reg &= ~PCI_EXP_LNKCTL2_TLS ; + +switch (pcie_link_speed[link_gen]) { +case PCIE_SPEED_2_5GT : + reg |=PCI_EXP_LNKCTL2_TLS_2_5GT ; +break ;Is this a style or indentation issue?
While writing this switch case, i observed couple of drivers doing like this(I dont remember the driver names). I checked now other PCIe controller drivers, break is inline to 'reg |=' line.
I will update it.
Quoted 11 lines
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 503
Quoted 4 lines
drivers/pci/controller/dwc/pcie-designware.c +3
-0+case PCIE_SPEED_5_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_5_0GT ; +break ;Ditto.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 506
Quoted 3 lines
drivers/pci/controller/dwc/pcie-designware.c +3
-0+case PCIE_SPEED_8_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_8_0GT ; +break ;Ditto.
Quoted 8 lines
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 509
Quoted 3 lines
drivers/pci/controller/dwc/pcie-designware.c +3
-0+case PCIE_SPEED_16_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_16_0GT ; +break ;Ditto.
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 511
Quoted 2 lines
drivers/pci/controller/dwc/pcie-designware.c +2
-0+default : +/* Use hardware capability */ Ditto.
I will correct it.
Quoted 8 lines
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 516
Quoted 6 lines
drivers/pci/controller/dwc/pcie-designware.c +5
-0+ val =dw_pcie_readl_dbi (pci, offset +PCI_EXP_LNKCAP ); + val =FIELD_GET (PCI_EXP_LNKCAP_SLS , val); + reg &= ~PCI_EXP_LNKCTL2_HASD ; + reg |=FIELD_PREP (PCI_EXP_LNKCTL2_TLS , val); +break ;Ditto.
Quoted 13 lines
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 529
Quoted 11 lines
drivers/pci/controller/dwc/pcie-designware.c +11
-0+ } + +dw_pcie_writel_dbi (pci, offset +PCI_EXP_LNKCTL2 , reg); +} +void dw_pcie_link_set_n_fts (struct dw_pcie *pci,u32 n_fts) +{ +u32 val; + + val =dw_pcie_readl_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL ); + val &= ~PORT_LOGIC_N_FTS ; + val |= n_fts;What if somebody supplies bits outside of the mask? I guess you need to apply proper masks to both values.
Agree, i will update it.
Quoted 5 lines
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci)
Quoted 4 lines
drivers/pci/controller/dwc/pcie-designware.c +3
-0+dw_pcie_writel_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL , val); +} +#define PORT_LOGIC_N_FTS GENMASK(7, 0)Shouldn't you use
_MASKsuffix here?
Agree, i will update it to PORT_LOGIC_N_FTS_MASK .
Quoted 3 lines
Quoted 2 lines
+#define
BUS_IATU_OFFSSZ_256MPerhaps less cryptic name?
I will update it to BUS_IATU_OFFSET
Quoted 5 lines
Quoted 2 lines
+#define
RST_INTRVL_DFT_MS100Less cryptic name would be
RESET_INTERVAL_MS
Agree, I will update it.
Quoted 11 lines
drivers/pci/controller/dwc/pcie-intel-gw.c line 90
Quoted 9 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +7
-0+static void pcie_update_bits (void __iomem *base,u32 mask,u32 val,u32 ofs) +{ +u32 old, new; + + old =readl (base + ofs); + new = old & ~mask; + new |= val & mask;Standard pattern
Quoted 3 lines
new = (old & ~mask) | (val & mask);And actually you may re-use 'val' variable and get rid of 'new' one.
Agree, will reuse the val:
val = (old & ~mask) | (val & mask);
Quoted 10 lines
drivers/pci/controller/dwc/pcie-intel-gw.c line 100
Quoted 9 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +8
-0+ +if (new != old) +writel (new, base + ofs); +} +static int intel_pcie_get_resources (struct platform_device *pdev) +{ + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"dbi" ); +No need to have this blank line.
Agree, will remove it.
Quoted 7 lines
drivers/pci/controller/dwc/pcie-intel-gw.c line 299
Quoted 6 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +5
-0+ pci->dbi_base =devm_ioremap_resource (dev, res); +if (IS_ERR (pci->dbi_base)) +return PTR_ERR (pci->dbi_base); + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"app" ); +Ditto.
Agree, will remove it.
Quoted 10 lines
drivers/pci/controller/dwc/pcie-intel-gw.c line 334
Quoted 9 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +8
-0+ lpp->app_base =devm_ioremap_resource (dev, res); +if (IS_ERR (lpp->app_base)) +return PTR_ERR (lpp->app_base); +return 0 ; +} +/* Read PMC status and wait for falling into L2 link state */ + ret = readl_poll_timeout(lpp->app_base + PCIE_APP_PMC, value, + (value & PCIE_APP_PMC_IN_L2), 20,Too many parentheses.
Agree, will remove it.
Quoted 8 lines
drivers/pci/controller/dwc/pcie-intel-gw.c line 374
Quoted 6 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +5
-0+ jiffies_to_usecs(5 * HZ)); +if (!lpp->pcie_cap_ofst) { + lpp->pcie_cap_ofst =dw_pcie_find_capability (&lpp->pci, +PCI_CAP_ID_EXP ); + }Wouldn't be slightly better to have something like
Quoted 7 lines
ret = dw_pcie_find_capability(&lpp->pci, PCI_CAP_ID_EXP); if (ret >= 0 && !lpp->pcie_cap_ofst) lpp->pcie_cap_ofst = ret;?
(It can be expanded to print error / warning messages if needed)
This function gets called during initialization and system resume. I have kept the if check to do dw_pcie_find_capability() only once.(not to do for system resume) For error check, i will update this to
if (!lpp->pcie_cap_ofst) { /*pcie_cap_ofst will be 0 for the very first time as kzalloc() is used for memory allocation for lpp structure*/ ret = dw_pcie_find_capability(&lpp->pci, PCI_CAP_ID_EXP);
if (!ret) { /* Function returns 0 in error case */
ret = -ENXIO;
dev_err();
goto app_init_err;
}
lpp->pcie_cap_ofst = ret;
}
Quoted 6 lines
drivers/pci/controller/dwc/pcie-intel-gw.c line 484
Quoted 4 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +3
-0+return ret; +} +platform_set_drvdata (pdev, lpp);I think it makes sense to setup at the end of the function (before
dev_info()call).
I have done it immediately after the memory allocation. Ok, i will move it before dev_info().
Quoted 5 lines
drivers/pci/controller/dwc/pcie-intel-gw.c line 497
Quoted 2 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +1
-0+ data =device_get_match_data (dev);Perhaps
if (!data) return -ENODEV; // -EINVAL?
Sure i will add it and ENODEV looks appropriate.
Quoted 7 lines
drivers/pci/controller/dwc/pcie-intel-gw.c line 513
Quoted 6 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +5
-0+/* +* Intel PCIe doesn't configure IO region, so set viewport +* to not to perform IO region access. +*/ + pci->num_viewport = data->num_viewport;Missed blank line?
I will add it.
Thanks a lot for reviewing and giving the inputs. I will update the new patch version according to your inputs and submit for review.
Regards,
Dilip
Quoted 2 lines
Quoted 2 lines
drivers/pci/controller/dwc/pcie-intel-gw.c new file +1
-0+dev_info (dev,"Intel PCIe Root Complex Port init done\n" );
Re: [PATCH v5 2/3] dwc: PCI: intel: PCIe RC controller driver
Quoted 3 lines · Dilip Kota
Quoted 1 line · Andy Shevchenko
On Wed, Nov 06, 2019 at 11:44:02AM +0800, Dilip Kota wrote:
[...]
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
+ return ret;
+}
+platform_set_drvdata(pdev, lpp);I think it makes sense to setup at the end of the function (before
dev_info()call).I have done it immediately after the memory allocation. Ok, i will move it before
dev_info().
I ran test with all the changes and kernel panic is hit due to NULL pointer access. It is because of platform_set_drvdata() moved before dev_info, which resulted in intel_pcie_get_resources() doing platform_get_drvdata and end up accessing NULL pointer. I will keep 'platform_set_drvdata()' remain unchanged.
Regards,
Dilip
Re: [PATCH v5 2/3] dwc: PCI: intel: PCIe RC controller driver
In reply to Dilip KotaQuoted 162 lines · Dilip Kota
Add support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.
Intel PCIe driver requires Upconfigure support, fast training sequence and link speed configuration. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it inpci_regs.h.Signed-off-by Dilip Kota <[email protected]>---
Changes on v5:
Use new, old variables inpcie_update_bits()
Correct naming:
s/Designware/DesignWare
s/pcie/PCIe
s/pci/PCI
s/Hw/HW
Upconfig to Upconfigure
Remove extra lines after theintel_pcie_max_speed_setup()def. Use pcie_link_speed[] and remove enum for pcie gen. Removedw_pcie_link_speed_change()def.
Remove "linux,pci-domain" parsing.
Remove 'id' variable inintel_pcie_portstructure.
Remove extra spaces forlpp->link_gen=
Correct the offset ofPCI_EXP_LNKCTL2_HASDto0x0020. Remove programming of RCB and CCC bits inPCI_EXP_LCTLreg. Remove programming Slot clock cfg inPCI_EXP_LSTSreg. Update the comments atnum_viewportsetting.
Update the description inKconfig.
Get PCIe cap offset from the registers usingdw_pcie_find_capability()Definepcie_cap_ofstvar inintel_pcie_portstruct to store the same. Remove thePCIE_CAP_OFSTmacro.
Moveintel_pcie_max_speed_setup()function to DesignWare framework,
defined asdw_pcie_link_set_max_speed()
SetEXPORT_SYMBOL_GPLfor the newer functions defined in
pcie-designware.cChanges on v4:
Rename the driver naming and description to
"PCIe RC controller on Intel Gateway SoCs".
Use PCIe core register macros defined inpci_regs.h
and remove respective local definitions.
Remove PCIe core interrupt handling.
Move PCIe link control speed change, upconfig and FTS. configuration functions to DesignWare framework.
Useof_pci_get_max_link_speed().
Mark dependency on X86 andCOMPILE_TESTinKconfig.
Remove lanes and addn_ftsvariables inintel_pcie_portstructure. Renamerst_intervalvariable torst_intrvlinintel_pcie_portstructure. Removeintel_pcie_mem_iatu()as it is already perfomed indw_setup_rc()Move sysfs attributes specific code to separate patch. Remove redundant error handling.
Reduce LoCs by doing variable initializations while declaration itself. Add extra line after closing parenthesis.
Moveintel_pcie_ep_rst_init()out ofget_resources()changes on v3:
Rename PCIe app logic registers withPCIE_APPprefix.PCIE_IOP_CTRLconfiguration is not required. Remove respective code. Remove wrapper functions for clk enable/disable APIs.
Useplatform_get_resource_byname()instead of
devm_platform_ioremap_resource()to be similar with DWC framework.
Rename phy name to "pciephy".
Modify debug message inmsi_init()callback to be more specific. Removemap_irq()callback.
Enable the INTx interrupts at the time of PCIe initialization. Reduce memory fragmentation by using variable "structdw_pciepci"
instead of allocating memory.
Reduce the delay to 100us during enpoint initialization
intel_pcie_ep_rst_init().
Calldw_pcie_host_deinit()duringremove()instead of directly
calling PCIe core APIs.
Rename "intel,rst-interval" to "reset-assert-ms".
Remove unused APP logic Interrupt bit macro definitions. Use dwc framework'sdw_pcie_setup_rc()for PCIe host specific
configuration instead of redefining the same functionality in the driver.
Move the whole DT parsing specific code tointel_pcie_get_resources()drivers/pci/controller/dwc/Kconfig | 10 + drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-designware.c | 57 +++ drivers/pci/controller/dwc/pcie-designware.h | 12 + drivers/pci/controller/dwc/pcie-intel-gw.c | 538 +++++++++++++++++++++++++++ include/uapi/linux/pci_regs.h | 1 + 6 files changed, 619 insertions(+) create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.cdrivers/pci/controller/dwc/Kconfig +10
-0@@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP order to enable device-specific features PCI_DW_PLAT_EP must be selected. +config PCIE_INTEL_GW + bool "Intel Gateway PCIe host controller support" + depends on OF && (X86 || COMPILE_TEST) + select PCIE_DW_HOST + help + Say 'Y' here to enable PCIe Host controller support on Intel + Gateway SoCs. + The PCIe controller uses the DesignWare core plus Intel-specific + hardware wrappers. + config PCI_EXYNOS bool "Samsung Exynos PCIe controller" depends on SOC_EXYNOS5440 || COMPILE_TESTdrivers/pci/controller/dwc/Makefile +1
-0@@ -3,6 +3,7 @@ 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 +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o obj-$(CONFIG_PCI_IMX6) += pci-imx6.odrivers/pci/controller/dwc/pcie-designware.c +25
-0@@ -14,6 +14,8 @@#include "pcie-designware.h" +extern const unsigned char pcie_link_speed[]; +/* * These interfaces resemble the pci_find_*capability() interfaces, but these * are for configuring host controllers, which are bridges *to* PCI devices but @@ -474,6 +476,61 @@ int dw_pcie_link_up(struct dw_pcie *pci) (!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING))); } +void dw_pcie_upconfig_setup (struct dw_pcie *pci) +{ +u32 val; + + val =dw_pcie_readl_dbi (pci,PCIE_PORT_MULTI_LANE_CTRL ); +dw_pcie_writel_dbi (pci,PCIE_PORT_MULTI_LANE_CTRL , + val |PORT_MLTI_UPCFG_SUPPORT ); +} +EXPORT_SYMBOL_GPL (dw_pcie_upconfig_setup); + +void dw_pcie_link_set_max_speed (struct dw_pcie *pci,u32 link_gen) +{ +u32 reg, val; +u8 offset =dw_pcie_find_capability (pci,PCI_CAP_ID_EXP ); + + reg =dw_pcie_readl_dbi (pci, offset +PCI_EXP_LNKCTL2 ); + reg &= ~PCI_EXP_LNKCTL2_TLS ; + +switch (pcie_link_speed[link_gen]) { +case PCIE_SPEED_2_5GT : + reg |=PCI_EXP_LNKCTL2_TLS_2_5GT ; +break ; +case PCIE_SPEED_5_0GT :
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 511
Quoted 10 lines
drivers/pci/controller/dwc/pcie-designware.c +10
-0+ reg |=PCI_EXP_LNKCTL2_TLS_5_0GT ; +break ; +case PCIE_SPEED_8_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_8_0GT ; +break ; +case PCIE_SPEED_16_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_16_0GT ; +break ; +default : +/* Use hardware capability */
I think the style here isn't quite inline with other controller drivers. Please indent the break statement.
Also the comment above should probably have the same indentation level as the code block.
But with those changes you can add:
Quoted 629 lines
drivers/pci/controller/dwc/pcie-designware.c +22
-0+ val =dw_pcie_readl_dbi (pci, offset +PCI_EXP_LNKCAP ); + val =FIELD_GET (PCI_EXP_LNKCAP_SLS , val); + reg &= ~PCI_EXP_LNKCTL2_HASD ; + reg |=FIELD_PREP (PCI_EXP_LNKCTL2_TLS , val); +break ; + } + +dw_pcie_writel_dbi (pci, offset +PCI_EXP_LNKCTL2 , reg); +} +EXPORT_SYMBOL_GPL (dw_pcie_link_set_max_speed); + +void dw_pcie_link_set_n_fts (struct dw_pcie *pci,u32 n_fts) +{ +u32 val; + + val =dw_pcie_readl_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL ); + val &= ~PORT_LOGIC_N_FTS ; + val |= n_fts; +dw_pcie_writel_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL , val); +} +EXPORT_SYMBOL_GPL (dw_pcie_link_set_n_fts); +static u8 dw_pcie_iatu_unroll_enabled (struct dw_pcie *pci) {u32 val;drivers/pci/controller/dwc/pcie-designware.h +12
-0@@ -30,7 +30,12 @@#define LINK_WAIT_IATU 9/* Synopsys-specific PCIe configuration registers */ +#define PCIE_PORT_AFR 0x70C +#define PORT_AFR_N_FTS_MASK GENMASK(15, 8) +#define PORT_AFR_CC_N_FTS_MASK GENMASK(23, 16) +#define PCIE_PORT_LINK_CONTROL 0x710 +#define PORT_LINK_DLL_LINK_EN BIT(5)#define PORT_LINK_MODE_MASK GENMASK(21, 16)#define PORT_LINK_MODE (n) FIELD_PREP(PORT_LINK_MODE_MASK, n)#define PORT_LINK_MODE_1_LANES PORT_LINK_MODE(0x1)@@ -46,6 +51,7 @@#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING BIT(29)#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C +#define PORT_LOGIC_N_FTS GENMASK(7, 0)#define PORT_LOGIC_SPEED_CHANGE BIT(17)#define PORT_LOGIC_LINK_WIDTH_MASK GENMASK(12, 8)#define PORT_LOGIC_LINK_WIDTH (n) FIELD_PREP(PORT_LOGIC_LINK_WIDTH_MASK, n)@@ -60,6 +66,9 @@#define PCIE_MSI_INTR0_MASK 0x82C#define PCIE_MSI_INTR0_STATUS 0x830 +#define PCIE_PORT_MULTI_LANE_CTRL 0x8C0 +#define PORT_MLTI_UPCFG_SUPPORT BIT(7) +#define PCIE_ATU_VIEWPORT 0x900#define PCIE_ATU_REGION_INBOUND BIT(31)#define PCIE_ATU_REGION_OUTBOUND 0@@ -273,6 +282,9 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);u32 dw_pcie_read_atu (struct dw_pcie *pci,u32 reg,size_t size);void dw_pcie_write_atu (struct dw_pcie *pci,u32 reg,size_t size,u32 val);int dw_pcie_link_up (struct dw_pcie *pci); +void dw_pcie_upconfig_setup (struct dw_pcie *pci); +void dw_pcie_link_set_max_speed (struct dw_pcie *pci,u32 link_gen); +void dw_pcie_link_set_n_fts (struct dw_pcie *pci,u32 n_fts);int dw_pcie_wait_for_link (struct dw_pcie *pci); void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type, u64 cpu_addr, u64 pci_addr,drivers/pci/controller/dwc/pcie-intel-gw.c new file +538
-0@@ -0,0 +1,538 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* PCIe host controller driver for Intel Gateway SoCs +* +* Copyright (c) 2019 Intel Corporation. +*/ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/gpio/consumer.h> +#include <linux/interrupt.h> +#include <linux/iopoll.h> +#include <linux/of_irq.h> +#include <linux/of_pci.h> +#include <linux/of_platform.h> +#include <linux/pci_regs.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/reset.h> + +#include "../../pci.h" +#include "pcie-designware.h" + +#define PORT_AFR_N_FTS_GEN12_DFT (SZ_128 - 1) +#define PORT_AFR_N_FTS_GEN3 180 +#define PORT_AFR_N_FTS_GEN4 196 + +/* PCIe Application logic Registers */ +#define PCIE_APP_CCR 0x10 +#define PCIE_APP_CCR_LTSSM_ENABLE BIT(0) + +#define PCIE_APP_MSG_CR 0x30 +#define PCIE_APP_MSG_XMT_PM_TURNOFF BIT(0) + +#define PCIE_APP_PMC 0x44 +#define PCIE_APP_PMC_IN_L2 BIT(20) + +#define PCIE_APP_IRNEN 0xF4 +#define PCIE_APP_IRNCR 0xF8 +#define PCIE_APP_IRN_AER_REPORT BIT(0) +#define PCIE_APP_IRN_PME BIT(2) +#define PCIE_APP_IRN_RX_VDM_MSG BIT(4) +#define PCIE_APP_IRN_PM_TO_ACK BIT(9) +#define PCIE_APP_IRN_LINK_AUTO_BW_STAT BIT(11) +#define PCIE_APP_IRN_BW_MGT BIT(12) +#define PCIE_APP_IRN_MSG_LTR BIT(18) +#define PCIE_APP_IRN_SYS_ERR_RC BIT(29) +#define PCIE_APP_INTX_OFST 12 + +#define PCIE_APP_IRN_INT \ + (PCIE_APP_IRN_AER_REPORT | PCIE_APP_IRN_PME | \ + PCIE_APP_IRN_RX_VDM_MSG | PCIE_APP_IRN_SYS_ERR_RC | \ + PCIE_APP_IRN_PM_TO_ACK | PCIE_APP_IRN_MSG_LTR | \ + PCIE_APP_IRN_BW_MGT | PCIE_APP_IRN_LINK_AUTO_BW_STAT | \ + (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTA) | \ + (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTB) | \ + (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTC) | \ + (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTD)) + +#define BUS_IATU_OFFS SZ_256M +#define RST_INTRVL_DFT_MS 100 + +struct intel_pcie_soc { +unsigned int pcie_ver; +unsigned int pcie_atu_offset; +u32 num_viewport; +}; + +struct intel_pcie_port { +struct dw_pcie pci; +void __iomem *app_base; +struct gpio_desc *reset_gpio; +u32 rst_intrvl; +u32 max_speed; +u32 link_gen; +u32 max_width; +u32 n_fts; +struct clk *core_clk; +struct reset_control *core_rst; +struct phy *phy; +u8 pcie_cap_ofst; +}; + +static void pcie_update_bits (void __iomem *base,u32 mask,u32 val,u32 ofs) +{ +u32 old, new; + + old =readl (base + ofs); + new = old & ~mask; + new |= val & mask; + +if (new != old) +writel (new, base + ofs); +} + +static inline u32 pcie_app_rd (struct intel_pcie_port *lpp,u32 ofs) +{ +return readl (lpp->app_base + ofs); +} + +static inline void pcie_app_wr (struct intel_pcie_port *lpp,u32 val,u32 ofs) +{ +writel (val, lpp->app_base + ofs); +} + +static void pcie_app_wr_mask (struct intel_pcie_port *lpp, +u32 mask,u32 val,u32 ofs) +{ +pcie_update_bits (lpp->app_base, mask, val, ofs); +} + +static inline u32 pcie_rc_cfg_rd (struct intel_pcie_port *lpp,u32 ofs) +{ +return dw_pcie_readl_dbi (&lpp->pci, ofs); +} + +static inline void pcie_rc_cfg_wr (struct intel_pcie_port *lpp,u32 val,u32 ofs) +{ +dw_pcie_writel_dbi (&lpp->pci, ofs, val); +} + +static void pcie_rc_cfg_wr_mask (struct intel_pcie_port *lpp, +u32 mask,u32 val,u32 ofs) +{ +pcie_update_bits (lpp->pci.dbi_base, mask, val, ofs); +} + +static void intel_pcie_ltssm_enable (struct intel_pcie_port *lpp) +{ +pcie_app_wr_mask (lpp,PCIE_APP_CCR_LTSSM_ENABLE , +PCIE_APP_CCR_LTSSM_ENABLE ,PCIE_APP_CCR ); +} + +static void intel_pcie_ltssm_disable (struct intel_pcie_port *lpp) +{ +pcie_app_wr_mask (lpp,PCIE_APP_CCR_LTSSM_ENABLE ,0 ,PCIE_APP_CCR ); +} + +static void intel_pcie_link_setup (struct intel_pcie_port *lpp) +{ +u32 val; +u8 offset = lpp->pcie_cap_ofst; + + val =pcie_rc_cfg_rd (lpp, offset +PCI_EXP_LNKCAP ); + lpp->max_speed =FIELD_GET (PCI_EXP_LNKCAP_SLS , val); + lpp->max_width =FIELD_GET (PCI_EXP_LNKCAP_MLW , val); + + val =pcie_rc_cfg_rd (lpp, offset +PCI_EXP_LNKCTL ); + + val &= ~(PCI_EXP_LNKCTL_LD |PCI_EXP_LNKCTL_ASPMC ); +pcie_rc_cfg_wr (lpp, val, offset +PCI_EXP_LNKCTL ); +} + +static void intel_pcie_port_logic_setup (struct intel_pcie_port *lpp) +{ +u32 val, mask; + +switch (pcie_link_speed[lpp->max_speed]) { +case PCIE_SPEED_8_0GT : + lpp->n_fts =PORT_AFR_N_FTS_GEN3 ; +break ; +case PCIE_SPEED_16_0GT : + lpp->n_fts =PORT_AFR_N_FTS_GEN4 ; +break ; +default : + lpp->n_fts =PORT_AFR_N_FTS_GEN12_DFT ; +break ; + } + + mask =PORT_AFR_N_FTS_MASK |PORT_AFR_CC_N_FTS_MASK ; + val =FIELD_PREP (PORT_AFR_N_FTS_MASK , lpp->n_fts) | +FIELD_PREP (PORT_AFR_CC_N_FTS_MASK , lpp->n_fts); +pcie_rc_cfg_wr_mask (lpp, mask, val,PCIE_PORT_AFR ); + +/* Port Link Control Register */ +pcie_rc_cfg_wr_mask (lpp,PORT_LINK_DLL_LINK_EN , +PORT_LINK_DLL_LINK_EN ,PCIE_PORT_LINK_CONTROL ); +} + +static void intel_pcie_rc_setup (struct intel_pcie_port *lpp) +{ +intel_pcie_ltssm_disable (lpp); +intel_pcie_link_setup (lpp); +dw_pcie_setup_rc (&lpp->pci.pp); +dw_pcie_upconfig_setup (&lpp->pci); +intel_pcie_port_logic_setup (lpp); +dw_pcie_link_set_max_speed (&lpp->pci, lpp->link_gen); +dw_pcie_link_set_n_fts (&lpp->pci, lpp->n_fts); +} + +static int intel_pcie_ep_rst_init (struct intel_pcie_port *lpp) +{ +struct device *dev = lpp->pci.dev; +int ret; + + lpp->reset_gpio =devm_gpiod_get (dev,"reset" ,GPIOD_OUT_LOW ); +if (IS_ERR (lpp->reset_gpio)) { + ret =PTR_ERR (lpp->reset_gpio); +if (ret != -EPROBE_DEFER ) +dev_err (dev,"failed to request PCIe GPIO: %d\n" , ret); +return ret; + } + +/* Make initial reset last for 100us */ +usleep_range (100 ,200 ); + +return 0 ; +} + +static void intel_pcie_core_rst_assert (struct intel_pcie_port *lpp) +{ +reset_control_assert (lpp->core_rst); +} + +static void intel_pcie_core_rst_deassert (struct intel_pcie_port *lpp) +{ +/* +* One micro-second delay to make sure the reset pulse +* wide enough so that core reset is clean. +*/ +udelay (1 ); +reset_control_deassert (lpp->core_rst); + +/* +* Some SoC core reset also reset PHY, more delay needed +* to make sure the reset process is done. +*/ +usleep_range (1000 ,2000 ); +} + +static void intel_pcie_device_rst_assert (struct intel_pcie_port *lpp) +{ +gpiod_set_value_cansleep (lpp->reset_gpio,1 ); +} + +static void intel_pcie_device_rst_deassert (struct intel_pcie_port *lpp) +{ +msleep (lpp->rst_intrvl); +gpiod_set_value_cansleep (lpp->reset_gpio,0 ); +} + +static int intel_pcie_app_logic_setup (struct intel_pcie_port *lpp) +{ +intel_pcie_device_rst_deassert (lpp); +intel_pcie_ltssm_enable (lpp); + +return dw_pcie_wait_for_link (&lpp->pci); +} + +static void intel_pcie_core_irq_disable (struct intel_pcie_port *lpp) +{ +pcie_app_wr (lpp,0 ,PCIE_APP_IRNEN ); +pcie_app_wr (lpp,PCIE_APP_IRN_INT ,PCIE_APP_IRNCR ); +} + +static int intel_pcie_get_resources (struct platform_device *pdev) +{ +struct intel_pcie_port *lpp =platform_get_drvdata (pdev); +struct dw_pcie *pci = &lpp->pci; +struct device *dev = pci->dev; +struct resource *res; +int ret; + + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"dbi" ); + + pci->dbi_base =devm_ioremap_resource (dev, res); +if (IS_ERR (pci->dbi_base)) +return PTR_ERR (pci->dbi_base); + + lpp->core_clk =devm_clk_get (dev,NULL ); +if (IS_ERR (lpp->core_clk)) { + ret =PTR_ERR (lpp->core_clk); +if (ret != -EPROBE_DEFER ) +dev_err (dev,"failed to get clks: %d\n" , ret); +return ret; + } + + lpp->core_rst =devm_reset_control_get (dev,NULL ); +if (IS_ERR (lpp->core_rst)) { + ret =PTR_ERR (lpp->core_rst); +if (ret != -EPROBE_DEFER ) +dev_err (dev,"failed to get resets: %d\n" , ret); +return ret; + } + + ret =device_property_match_string (dev,"device_type" ,"pci" ); +if (ret) { +dev_err (dev,"failed to find pci device type: %d\n" , ret); +return ret; + } + +if (device_property_read_u32 (dev,"reset-assert-ms" , &lpp->rst_intrvl)) + lpp->rst_intrvl =RST_INTRVL_DFT_MS ; + + ret =of_pci_get_max_link_speed (dev->of_node); + lpp->link_gen = ret <0 ?0 : ret; + + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"app" ); + + lpp->app_base =devm_ioremap_resource (dev, res); +if (IS_ERR (lpp->app_base)) +return PTR_ERR (lpp->app_base); + + lpp->phy =devm_phy_get (dev,"pcie" ); +if (IS_ERR (lpp->phy)) { + ret =PTR_ERR (lpp->phy); +if (ret != -EPROBE_DEFER ) +dev_err (dev,"couldn't get pcie-phy: %d\n" , ret); +return ret; + } + +return 0 ; +} + +static void intel_pcie_deinit_phy (struct intel_pcie_port *lpp) +{ +phy_exit (lpp->phy); +} + +static int intel_pcie_wait_l2 (struct intel_pcie_port *lpp) +{ +u32 value; +int ret; + +if (pcie_link_speed[lpp->max_speed] <PCIE_SPEED_8_0GT ) +return 0 ; + +/* Send PME_TURN_OFF message */ +pcie_app_wr_mask (lpp,PCIE_APP_MSG_XMT_PM_TURNOFF , +PCIE_APP_MSG_XMT_PM_TURNOFF ,PCIE_APP_MSG_CR ); + +/* Read PMC status and wait for falling into L2 link state */ + ret =readl_poll_timeout (lpp->app_base +PCIE_APP_PMC , value, + (value &PCIE_APP_PMC_IN_L2 ),20 , +jiffies_to_usecs (5 *HZ )); +if (ret) +dev_err (lpp->pci.dev,"PCIe link enter L2 timeout!\n" ); + +return ret; +} + +static void intel_pcie_turn_off (struct intel_pcie_port *lpp) +{ +if (dw_pcie_link_up (&lpp->pci)) +intel_pcie_wait_l2 (lpp); + +/* Put endpoint device in reset state */ +intel_pcie_device_rst_assert (lpp); +pcie_rc_cfg_wr_mask (lpp,PCI_COMMAND_MEMORY ,0 ,PCI_COMMAND ); +} + +static int intel_pcie_host_setup (struct intel_pcie_port *lpp) +{ +int ret; + +intel_pcie_core_rst_assert (lpp); +intel_pcie_device_rst_assert (lpp); + + ret =phy_init (lpp->phy); +if (ret) +return ret; + +intel_pcie_core_rst_deassert (lpp); + + ret =clk_prepare_enable (lpp->core_clk); +if (ret) { +dev_err (lpp->pci.dev,"Core clock enable failed: %d\n" , ret); + goto clk_err; + } + +if (!lpp->pcie_cap_ofst) { + lpp->pcie_cap_ofst =dw_pcie_find_capability (&lpp->pci, +PCI_CAP_ID_EXP ); + } + +intel_pcie_rc_setup (lpp); + ret =intel_pcie_app_logic_setup (lpp); +if (ret) + goto app_init_err; + +/* Enable integrated interrupts */ +pcie_app_wr_mask (lpp,PCIE_APP_IRN_INT ,PCIE_APP_IRN_INT , +PCIE_APP_IRNEN ); +return 0 ; + +app_init_err: +clk_disable_unprepare (lpp->core_clk); +clk_err: +intel_pcie_core_rst_assert (lpp); +intel_pcie_deinit_phy (lpp); + +return ret; +} + +static void __intel_pcie_remove (struct intel_pcie_port *lpp) +{ +intel_pcie_core_irq_disable (lpp); +intel_pcie_turn_off (lpp); +clk_disable_unprepare (lpp->core_clk); +intel_pcie_core_rst_assert (lpp); +intel_pcie_deinit_phy (lpp); +} + +static int intel_pcie_remove (struct platform_device *pdev) +{ +struct intel_pcie_port *lpp =platform_get_drvdata (pdev); +struct pcie_port *pp = &lpp->pci.pp; + +dw_pcie_host_deinit (pp); +__intel_pcie_remove (lpp); + +return 0 ; +} + +static int __maybe_unused intel_pcie_suspend_noirq (struct device *dev) +{ +struct intel_pcie_port *lpp =dev_get_drvdata (dev); +int ret; + +intel_pcie_core_irq_disable (lpp); + ret =intel_pcie_wait_l2 (lpp); +if (ret) +return ret; + +intel_pcie_deinit_phy (lpp); +clk_disable_unprepare (lpp->core_clk); +return ret; +} + +static int __maybe_unused intel_pcie_resume_noirq (struct device *dev) +{ +struct intel_pcie_port *lpp =dev_get_drvdata (dev); + +return intel_pcie_host_setup (lpp); +} + +static int intel_pcie_rc_init (struct pcie_port *pp) +{ +struct dw_pcie *pci =to_dw_pcie_from_pp (pp); +struct intel_pcie_port *lpp =dev_get_drvdata (pci->dev); + +return intel_pcie_host_setup (lpp); +} + +int intel_pcie_msi_init (struct pcie_port *pp) +{ +/* PCIe MSI/MSIx is handled by MSI in x86 processor */ +return 0 ; +} + +u64 intel_pcie_cpu_addr (struct dw_pcie *pcie,u64 cpu_addr) +{ +return cpu_addr +BUS_IATU_OFFS ; +} + +static const struct dw_pcie_ops intel_pcie_ops = { + .cpu_addr_fixup = intel_pcie_cpu_addr, +}; + +static const struct dw_pcie_host_ops intel_pcie_dw_ops = { + .host_init = intel_pcie_rc_init, + .msi_host_init = intel_pcie_msi_init, +}; + +static const struct intel_pcie_soc pcie_data = { + .pcie_ver =0x520A , + .pcie_atu_offset =0xC0000 , + .num_viewport =3 , +}; + +static int intel_pcie_probe (struct platform_device *pdev) +{ +const struct intel_pcie_soc *data; +struct device *dev = &pdev->dev; +struct intel_pcie_port *lpp; +struct pcie_port *pp; +struct dw_pcie *pci; +int ret; + + lpp =devm_kzalloc (dev,sizeof (*lpp),GFP_KERNEL ); +if (!lpp) +return -ENOMEM ; + +platform_set_drvdata (pdev, lpp); + pci = &lpp->pci; + pci->dev = dev; + pp = &pci->pp; + + ret =intel_pcie_get_resources (pdev); +if (ret) +return ret; + + ret =intel_pcie_ep_rst_init (lpp); +if (ret) +return ret; + + data =device_get_match_data (dev); + pci->ops = &intel_pcie_ops; + pci->version = data->pcie_ver; + pci->atu_base = pci->dbi_base + data->pcie_atu_offset; + pp->ops = &intel_pcie_dw_ops; + + ret =dw_pcie_host_init (pp); +if (ret) { +dev_err (dev,"cannot initialize host\n" ); +return ret; + } + +/* +* Intel PCIe doesn't configure IO region, so set viewport +* to not to perform IO region access. +*/ + pci->num_viewport = data->num_viewport; +dev_info (dev,"Intel PCIe Root Complex Port init done\n" ); + +return ret; +} + +static const struct dev_pm_ops intel_pcie_pm_ops = { +SET_NOIRQ_SYSTEM_SLEEP_PM_OPS (intel_pcie_suspend_noirq, + intel_pcie_resume_noirq) +}; + +static const struct of_device_id of_intel_pcie_match[] = { + { .compatible ="intel,lgm-pcie" , .data = &pcie_data }, + {} +}; + +static struct platform_driver intel_pcie_driver = { + .probe = intel_pcie_probe, + .remove = intel_pcie_remove, + .driver = { + .name ="intel-gw-pcie" , + .of_match_table = of_intel_pcie_match, + .pm = &intel_pcie_pm_ops, + }, +}; +builtin_platform_driver (intel_pcie_driver);include/uapi/linux/pci_regs.h +1
-0@@ -673,6 +673,7 @@#define PCI_EXP_LNKCTL2_TLS_8_0GT 0x0003/* Supported Speed 8GT/s */ #define PCI_EXP_LNKCTL2_TLS_16_0GT 0x0004/* Supported Speed 16GT/s */ #define PCI_EXP_LNKCTL2_TLS_32_0GT 0x0005/* Supported Speed 32GT/s */ +#define PCI_EXP_LNKCTL2_HASD 0x0020/* HW Autonomous Speed Disable */ #define PCI_EXP_LNKSTA2 50/* Link Status 2 */ #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 52/* v2 endpoints with link end here */ #define PCI_EXP_SLTCAP2 52/* Slot Capabilities 2 */ Signature · 1 line
2.11.0
Re: [PATCH v5 2/3] dwc: PCI: intel: PCIe RC controller driver
Quoted 167 lines · Andrew Murray
Quoted 166 lines · Dilip Kota
Add support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.
Intel PCIe driver requires Upconfigure support, fast training sequence and link speed configuration. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it inpci_regs.h.Signed-off-by Dilip Kota <[email protected]>---
Changes on v5:
Use new, old variables inpcie_update_bits()
Correct naming:
s/Designware/DesignWare
s/pcie/PCIe
s/pci/PCI
s/Hw/HW
Upconfig to Upconfigure
Remove extra lines after theintel_pcie_max_speed_setup()def. Use pcie_link_speed[] and remove enum for pcie gen. Removedw_pcie_link_speed_change()def.
Remove "linux,pci-domain" parsing.
Remove 'id' variable inintel_pcie_portstructure.
Remove extra spaces forlpp->link_gen=
Correct the offset ofPCI_EXP_LNKCTL2_HASDto0x0020. Remove programming of RCB and CCC bits inPCI_EXP_LCTLreg. Remove programming Slot clock cfg inPCI_EXP_LSTSreg. Update the comments atnum_viewportsetting.
Update the description inKconfig.
Get PCIe cap offset from the registers usingdw_pcie_find_capability()Definepcie_cap_ofstvar inintel_pcie_portstruct to store the same. Remove thePCIE_CAP_OFSTmacro.
Moveintel_pcie_max_speed_setup()function to DesignWare framework,
defined asdw_pcie_link_set_max_speed()
SetEXPORT_SYMBOL_GPLfor the newer functions defined in
pcie-designware.cChanges on v4:
Rename the driver naming and description to
"PCIe RC controller on Intel Gateway SoCs".
Use PCIe core register macros defined inpci_regs.h
and remove respective local definitions.
Remove PCIe core interrupt handling.
Move PCIe link control speed change, upconfig and FTS. configuration functions to DesignWare framework.
Useof_pci_get_max_link_speed().
Mark dependency on X86 andCOMPILE_TESTinKconfig.
Remove lanes and addn_ftsvariables inintel_pcie_portstructure. Renamerst_intervalvariable torst_intrvlinintel_pcie_portstructure. Removeintel_pcie_mem_iatu()as it is already perfomed indw_setup_rc()Move sysfs attributes specific code to separate patch. Remove redundant error handling.
Reduce LoCs by doing variable initializations while declaration itself. Add extra line after closing parenthesis.
Moveintel_pcie_ep_rst_init()out ofget_resources()changes on v3:
Rename PCIe app logic registers withPCIE_APPprefix.PCIE_IOP_CTRLconfiguration is not required. Remove respective code. Remove wrapper functions for clk enable/disable APIs.
Useplatform_get_resource_byname()instead of
devm_platform_ioremap_resource()to be similar with DWC framework.
Rename phy name to "pciephy".
Modify debug message inmsi_init()callback to be more specific. Removemap_irq()callback.
Enable the INTx interrupts at the time of PCIe initialization. Reduce memory fragmentation by using variable "structdw_pciepci"
instead of allocating memory.
Reduce the delay to 100us during enpoint initialization
intel_pcie_ep_rst_init().
Calldw_pcie_host_deinit()duringremove()instead of directly
calling PCIe core APIs.
Rename "intel,rst-interval" to "reset-assert-ms".
Remove unused APP logic Interrupt bit macro definitions. Use dwc framework'sdw_pcie_setup_rc()for PCIe host specific
configuration instead of redefining the same functionality in the driver.
Move the whole DT parsing specific code tointel_pcie_get_resources()drivers/pci/controller/dwc/Kconfig | 10 + drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-designware.c | 57 +++ drivers/pci/controller/dwc/pcie-designware.h | 12 + drivers/pci/controller/dwc/pcie-intel-gw.c | 538 +++++++++++++++++++++++++++ include/uapi/linux/pci_regs.h | 1 + 6 files changed, 619 insertions(+)create mode 100644
drivers/pci/controller/dwc/pcie-intel-gw.cdrivers/pci/controller/dwc/Kconfig +10
-0@@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP order to enable device-specific features PCI_DW_PLAT_EP must be selected. +config PCIE_INTEL_GW + bool "Intel Gateway PCIe host controller support" + depends on OF && (X86 || COMPILE_TEST) + select PCIE_DW_HOST + help + Say 'Y' here to enable PCIe Host controller support on Intel + Gateway SoCs. + The PCIe controller uses the DesignWare core plus Intel-specific + hardware wrappers. + config PCI_EXYNOS bool "Samsung Exynos PCIe controller" depends on SOC_EXYNOS5440 || COMPILE_TESTdrivers/pci/controller/dwc/Makefile +1
-0@@ -3,6 +3,7 @@ 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 +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o obj-$(CONFIG_PCI_IMX6) += pci-imx6.odrivers/pci/controller/dwc/pcie-designware.c +29
-0@@ -14,6 +14,8 @@#include "pcie-designware.h" +extern const unsigned char pcie_link_speed[]; +/* * These interfaces resemble the pci_find_*capability() interfaces, but these * are for configuring host controllers, which are bridges *to* PCI devices but @@ -474,6 +476,61 @@ int dw_pcie_link_up(struct dw_pcie *pci) (!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING))); } +void dw_pcie_upconfig_setup (struct dw_pcie *pci) +{ +u32 val; + + val =dw_pcie_readl_dbi (pci,PCIE_PORT_MULTI_LANE_CTRL ); +dw_pcie_writel_dbi (pci,PCIE_PORT_MULTI_LANE_CTRL , + val |PORT_MLTI_UPCFG_SUPPORT ); +} +EXPORT_SYMBOL_GPL (dw_pcie_upconfig_setup); + +void dw_pcie_link_set_max_speed (struct dw_pcie *pci,u32 link_gen) +{ +u32 reg, val; +u8 offset =dw_pcie_find_capability (pci,PCI_CAP_ID_EXP ); + + reg =dw_pcie_readl_dbi (pci, offset +PCI_EXP_LNKCTL2 ); + reg &= ~PCI_EXP_LNKCTL2_TLS ; + +switch (pcie_link_speed[link_gen]) { +case PCIE_SPEED_2_5GT : + reg |=PCI_EXP_LNKCTL2_TLS_2_5GT ; +break ; +case PCIE_SPEED_5_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_5_0GT ; +break ; +case PCIE_SPEED_8_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_8_0GT ;
Quoted 8 lines
drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_link_up(struct dw_pcie *pci) · line 511
Quoted 6 lines
drivers/pci/controller/dwc/pcie-designware.c +6
-0+break ; +case PCIE_SPEED_16_0GT : + reg |=PCI_EXP_LNKCTL2_TLS_16_0GT ; +break ; +default : +/* Use hardware capability */ I think the style here isn't quite inline with other controller drivers. Please indent the break statement.
While writing this switch case, i observed couple of drivers doing like this(I dont remember the driver names). I checked now other PCIe controller drivers, break is inline to 'reg |=' line.
I will update it.
Quoted 3 lines
Also the comment above should probably have the same indentation level as the code block.
Sure, i will correct it.
Quoted 4 lines
But with those changes you can add:
Reviewed-by Andrew Murray <[email protected]>
Thanks a lot for reviewing patch and giving the inputs.
Regards,
Dilip
PATCH v5 3/3 PCI: artpec6: Configure FTS with dwc helper function
In reply to Dilip KotaUtilize DesugnWare helper functions to configure Fast Training Sequence. Drop the respective code in the driver.
--- drivers/pci/controller/dwc/pcie-artpec6.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/pci/controller/dwc/pcie-artpec6.c +1 -7
@@ -51,9 +51,6 @@ static const struct of_device_id artpec6_pcie_of_match[];#define ACK_N_FTS_MASK GENMASK(15, 8)#define ACK_N_FTS (x) (((x) << 8) & ACK_N_FTS_MASK) -#define FAST_TRAINING_SEQ_MASK GENMASK(7, 0) -#define FAST_TRAINING_SEQ (x) (((x) << 0) & FAST_TRAINING_SEQ_MASK) -/* ARTPEC-6 specific registers */ #define PCIECFG 0x18#define PCIECFG_DBG_OEN BIT(24)
@@ -313,10 +310,7 @@ static void artpec6_pcie_set_nfts(struct artpec6_pcie *artpec6_pcie)* Set the Number of Fast Training Sequences that the core * advertises as its N_FTS during Gen2 or Gen3 link training. */ - val =dw_pcie_readl_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL ); - val &= ~FAST_TRAINING_SEQ_MASK ; - val |=FAST_TRAINING_SEQ (180 ); -dw_pcie_writel_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL , val); +dw_pcie_link_set_n_fts (pci,180 ); }static void artpec6_pcie_assert_core_reset (struct artpec6_pcie *artpec6_pcie)
Signature · 1 line
2.11.0
RE: [PATCH v5 3/3] PCI: artpec6: Configure FTS with dwc helper function
Quoted 2 lines · Dilip Kota
Utilize DesugnWare helper functions to configure Fast Training Sequence. Drop the respective code in the driver.
Please fix
s/DesugnWare/DesignWare
Quoted 34 lines
Signed-off-by Dilip Kota <[email protected]>--- drivers/pci/controller/dwc/pcie-artpec6.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)drivers/pci/controller/dwc/pcie-artpec6.c +1
-7@@ -51,9 +51,6 @@ static const struct of_device_id artpec6_pcie_of_match[];#define ACK_N_FTS_MASK GENMASK(15, 8)#define ACK_N_FTS (x) (((x) << 8) & ACK_N_FTS_MASK) -#define FAST_TRAINING_SEQ_MASK GENMASK(7, 0) -#define FAST_TRAINING_SEQ (x) (((x) << 0) & FAST_TRAINING_SEQ_MASK) -/* ARTPEC-6 specific registers */ #define PCIECFG 0x18#define PCIECFG_DBG_OEN BIT(24)@@ -313,10 +310,7 @@ static void artpec6_pcie_set_nfts(struct artpec6_pcie *artpec6_pcie)* Set the Number of Fast Training Sequences that the core * advertises as its N_FTS during Gen2 or Gen3 link training. */ - val =dw_pcie_readl_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL ); - val &= ~FAST_TRAINING_SEQ_MASK ; - val |=FAST_TRAINING_SEQ (180 ); -dw_pcie_writel_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL , val); +dw_pcie_link_set_n_fts (pci,180 ); }static void artpec6_pcie_assert_core_reset (struct artpec6_pcie *artpec6_pcie)Signature · 1 line
2.11.0
Re: [PATCH v5 3/3] PCI: artpec6: Configure FTS with dwc helper function
Quoted 7 lines · Gustavo Pimentel
Quoted 2 lines · Dilip Kota
Utilize DesugnWare helper functions to configure Fast Training Sequence. Drop the respective code in the driver.
Please fix
s/DesugnWare/DesignWare
Typo error. I will correct it in the next patch version.
Thanks for reviewing it.
Regards,
Dilip
Jingoo Han
maintainerRe: [PATCH v5 3/3] PCI: artpec6: Configure FTS with dwc helper function
In reply to Dilip KotaQuoted 2 lines · Dilip Kota
Utilize DesugnWare helper functions to configure Fast Training
Nitpicking: Fix typo (DesugnWare --> DesignWare)
If possible, how about the following?
Utilize DesignWare --> Use DesignWare
Best regards,
Jingoo Han
Quoted 35 lines
Sequence. Drop the respective code in the driver.
Signed-off-by Dilip Kota <[email protected]>--- drivers/pci/controller/dwc/pcie-artpec6.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)drivers/pci/controller/dwc/pcie-artpec6.c +1
-7@@ -51,9 +51,6 @@ static const struct of_device_id artpec6_pcie_of_match[];#define ACK_N_FTS_MASK GENMASK(15, 8)#define ACK_N_FTS (x) (((x) << 8) & ACK_N_FTS_MASK) -#define FAST_TRAINING_SEQ_MASK GENMASK(7, 0) -#define FAST_TRAINING_SEQ (x) (((x) << 0) & FAST_TRAINING_SEQ_MASK) -/* ARTPEC-6 specific registers */ #define PCIECFG 0x18#define PCIECFG_DBG_OEN BIT(24)@@ -313,10 +310,7 @@ static void artpec6_pcie_set_nfts(struct artpec6_pcie *artpec6_pcie)* Set the Number of Fast Training Sequences that the core * advertises as its N_FTS during Gen2 or Gen3 link training. */ - val =dw_pcie_readl_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL ); - val &= ~FAST_TRAINING_SEQ_MASK ; - val |=FAST_TRAINING_SEQ (180 ); -dw_pcie_writel_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL , val); +dw_pcie_link_set_n_fts (pci,180 ); }static void artpec6_pcie_assert_core_reset (struct artpec6_pcie *artpec6_pcie)Signature · 1 line
2.11.0
Andrew Murray
Reviewed-byRe: [PATCH v5 3/3] PCI: artpec6: Configure FTS with dwc helper function
Quoted 4 lines · Jingoo Han
Quoted 2 lines · Dilip Kota
Utilize DesugnWare helper functions to configure Fast Training
Quoted 11 lines
Nitpicking: Fix typo (DesugnWare --> DesignWare)
If possible, how about the following?
Utilize DesignWare --> Use DesignWareBest regards,
Jingoo HanQuoted 3 lines
Sequence. Drop the respective code in the driver.
Signed-off-by Dilip Kota <[email protected]>
With the changes suggested in this thread, you can add:
Quoted 33 lines
Quoted 32 lines
--- drivers/pci/controller/dwc/pcie-artpec6.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)drivers/pci/controller/dwc/pcie-artpec6.c +1
-7@@ -51,9 +51,6 @@ static const struct of_device_id artpec6_pcie_of_match[];#define ACK_N_FTS_MASK GENMASK(15, 8)#define ACK_N_FTS (x) (((x) << 8) & ACK_N_FTS_MASK) -#define FAST_TRAINING_SEQ_MASK GENMASK(7, 0) -#define FAST_TRAINING_SEQ (x) (((x) << 0) & FAST_TRAINING_SEQ_MASK) -/* ARTPEC-6 specific registers */ #define PCIECFG 0x18#define PCIECFG_DBG_OEN BIT(24)@@ -313,10 +310,7 @@ static void artpec6_pcie_set_nfts(struct artpec6_pcie *artpec6_pcie)* Set the Number of Fast Training Sequences that the core * advertises as its N_FTS during Gen2 or Gen3 link training. */ - val =dw_pcie_readl_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL ); - val &= ~FAST_TRAINING_SEQ_MASK ; - val |=FAST_TRAINING_SEQ (180 ); -dw_pcie_writel_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL , val); +dw_pcie_link_set_n_fts (pci,180 ); }static void artpec6_pcie_assert_core_reset (struct artpec6_pcie *artpec6_pcie)Signature · 1 line
2.11.0
Re: [PATCH v5 3/3] PCI: artpec6: Configure FTS with dwc helper function
Quoted 11 lines · Andrew Murray
Quoted 10 lines · Jingoo Han
Quoted 1 line · Dilip Kota
Utilize DesugnWare helper functions to configure Fast Training
Nitpicking: Fix typo (DesugnWare --> DesignWare)
If possible, how about the following?
Utilize DesignWare --> Use DesignWareBest regards,
Jingoo Han
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Sequence. Drop the respective code in the driver.
Signed-off-by Dilip Kota <[email protected]>With the changes suggested in this thread, you can add:
Reviewed-by Andrew Murray <[email protected]>
Sure.
Thanks a lot for reviewing patch and giving inputs,
Regards,
Dilip
Quoted 33 lines
Quoted 33 lines
Quoted 33 lines
--- drivers/pci/controller/dwc/pcie-artpec6.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)drivers/pci/controller/dwc/pcie-artpec6.c +1
-7@@ -51,9 +51,6 @@ static const struct of_device_id artpec6_pcie_of_match[];#define ACK_N_FTS_MASK GENMASK(15, 8)#define ACK_N_FTS (x) (((x) << 8) & ACK_N_FTS_MASK) -#define FAST_TRAINING_SEQ_MASK GENMASK(7, 0) -#define FAST_TRAINING_SEQ (x) (((x) << 0) & FAST_TRAINING_SEQ_MASK) -/* ARTPEC-6 specific registers */ #define PCIECFG 0x18#define PCIECFG_DBG_OEN BIT(24)@@ -313,10 +310,7 @@ static void artpec6_pcie_set_nfts(struct artpec6_pcie *artpec6_pcie)* Set the Number of Fast Training Sequences that the core * advertises as its N_FTS during Gen2 or Gen3 link training. */ - val =dw_pcie_readl_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL ); - val &= ~FAST_TRAINING_SEQ_MASK ; - val |=FAST_TRAINING_SEQ (180 ); -dw_pcie_writel_dbi (pci,PCIE_LINK_WIDTH_SPEED_CONTROL , val); +dw_pcie_link_set_n_fts (pci,180 ); }static void artpec6_pcie_assert_core_reset (struct artpec6_pcie *artpec6_pcie)Signature · 1 line
2.11.0
Re: [PATCH v5 3/3] PCI: artpec6: Configure FTS with dwc helper function
In reply to Jingoo HanQuoted 6 lines · Jingoo Han
Quoted 1 line · Dilip Kota
Utilize DesugnWare helper functions to configure Fast Training
Nitpicking: Fix typo (DesugnWare --> DesignWare)
If possible, how about the following?
Utilize DesignWare --> Use DesignWare
Sure, i will correct it and update to 'Use DesignWare'
Thanks for reviewing the patch,
Regards,
Dilip
Quoted 4 lines
Best regards,
Jingoo Han