lkml.org 
[lkml]   [2019]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v5 2/3] dwc: PCI: intel: PCIe RC controller driver
From
Date

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


\
 
 \ /
  Last update: 2019-11-11 09:10    [W:0.620 / U:1.292 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site