Messages in this thread | | | Subject | Re: [PATCH v5 2/3] dwc: PCI: intel: PCIe RC controller driver | From | Dilip Kota <> | Date | Mon, 11 Nov 2019 16:09:57 +0800 |
| |
On 11/8/2019 6:42 PM, Andrew Murray wrote: > On Wed, Nov 06, 2019 at 11:44:02AM +0800, Dilip Kota wrote: >> 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 in pci_regs.h. >> >> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com> >> --- >> 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 >> >> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig >> index 0ba988b5b5bc..39c17b16b403 100644 >> --- a/drivers/pci/controller/dwc/Kconfig >> +++ b/drivers/pci/controller/dwc/Kconfig >> @@ -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 >> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile >> index b30336181d46..99db83cd2f35 100644 >> --- a/drivers/pci/controller/dwc/Makefile >> +++ b/drivers/pci/controller/dwc/Makefile >> @@ -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 >> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c >> index 820488dfeaed..20e5c3aec394 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware.c >> +++ b/drivers/pci/controller/dwc/pcie-designware.c >> @@ -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; >> + 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. > > Also the comment above should probably have the same indentation level > as the code block. Sure, i will correct it. > > But with those changes you can add: > > Reviewed-by: Andrew Murray <andrew.murray@arm.con> Thanks a lot for reviewing patch and giving the inputs.
Regards, Dilip
| |