| Message ID | 20240301051220.20917-2-quic_schintav@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Gen4 equalization and margining settings | expand |
tag should be PCI: qcom: Frank On Thu, Feb 29, 2024 at 09:11:34PM -0800, Shashank Babu Chinta Venkata wrote: > Refactor common code from RC(Root Complex) and EP(End Point) > drivers and move them to a common repository. This acts as placeholder > for common source code for both drivers avoiding duplication. > > Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com> > --- > drivers/pci/controller/dwc/Kconfig | 5 ++ > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-qcom-cmn.c | 85 ++++++++++++++++++++++ > drivers/pci/controller/dwc/pcie-qcom-cmn.h | 30 ++++++++ > drivers/pci/controller/dwc/pcie-qcom-ep.c | 39 +--------- > drivers/pci/controller/dwc/pcie-qcom.c | 67 ++--------------- > 6 files changed, 133 insertions(+), 94 deletions(-) > create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.c > create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.h > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index 8afacc90c63b..41d2746edc5f 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -265,12 +265,16 @@ config PCIE_DW_PLAT_EP > order to enable device-specific features PCI_DW_PLAT_EP must be > selected. > > +config PCIE_QCOM_CMN > + bool > + > config PCIE_QCOM > bool "Qualcomm PCIe controller (host mode)" > depends on OF && (ARCH_QCOM || COMPILE_TEST) > depends on PCI_MSI > select PCIE_DW_HOST > select CRC8 > + select PCIE_QCOM_CMN > help > Say Y here to enable PCIe controller support on Qualcomm SoCs. The > PCIe controller uses the DesignWare core plus Qualcomm-specific > @@ -281,6 +285,7 @@ config PCIE_QCOM_EP > depends on OF && (ARCH_QCOM || COMPILE_TEST) > depends on PCI_ENDPOINT > select PCIE_DW_EP > + select PCIE_QCOM_CMN > help > Say Y here to enable support for the PCIe controllers on Qualcomm SoCs > to work in endpoint mode. The PCIe controller uses the DesignWare core > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > index bac103faa523..022dc73c38a5 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -27,6 +27,7 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o > obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o > obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o > obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o > +obj-$(CONFIG_PCIE_QCOM_CMN) += pcie-qcom-cmn.o > > # The following drivers are for devices that use the generic ACPI > # pci_root.c driver but don't support standard ECAM config access. > diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.c b/drivers/pci/controller/dwc/pcie-qcom-cmn.c > new file mode 100644 > index 000000000000..0f8d004fbc79 > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c > @@ -0,0 +1,85 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved. > + * Copyright 2015, 2021 Linaro Limited. > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > + * > + */ > + > +#include <linux/debugfs.h> > +#include <linux/pci.h> > +#include <linux/interconnect.h> > + > +#include "../../pci.h" > +#include "pcie-designware.h" > +#include "pcie-qcom-cmn.h" > + > + > +#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \ > + Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed])) > + > + > +int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem) > +{ > + int ret = 0; > + > + if (IS_ERR(pci)) > + return PTR_ERR(pci); > + > + icc_mem = devm_of_icc_get(pci->dev, "pcie-mem"); > + if (IS_ERR(icc_mem)) > + return PTR_ERR(icc_mem); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_get_resource); > + > +int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem) > +{ > + int ret = 0; > + > + if (IS_ERR(pci)) > + return PTR_ERR(pci); > + > + if (IS_ERR(icc_mem)) > + return PTR_ERR(icc_mem); > + > + /* > + * Some Qualcomm platforms require interconnect bandwidth constraints > + * to be set before enabling interconnect clocks. > + * > + * Set an initial peak bandwidth corresponding to single-lane Gen 1 > + * for the pcie-mem path. > + */ > + ret = icc_set_bw(icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1)); > + if (ret) { > + dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", > + ret); > + return ret; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_init); > + > +void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem) > +{ > + u32 offset, status; > + int speed, width; > + int ret; > + > + if (!icc_mem) > + return; > + > + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); > + status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA); > + > + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status); > + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status); > + > + ret = icc_set_bw(icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed)); > + if (ret) > + dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", > + ret); > +} > +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_update); > diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.h b/drivers/pci/controller/dwc/pcie-qcom-cmn.h > new file mode 100644 > index 000000000000..8794dbd4775c > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h > @@ -0,0 +1,30 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved. > + * Copyright 2015, 2021 Linaro Limited. > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/pci.h> > +#include "../../pci.h" > +#include "pcie-designware.h" > + > +#ifdef CONFIG_PCIE_QCOM_CMN > +int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem); > +int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem); > +void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem); > +#else > +static inline int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem) > +{ > + return 0; > +} > + > +static inline int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem) > +{ > + return 0; > +} > + > +static inline void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem) > +{ > +} > +#endif > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c > index 36e5e80cd22f..ce6343426de8 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c > @@ -25,6 +25,7 @@ > > #include "../../pci.h" > #include "pcie-designware.h" > +#include "pcie-qcom-cmn.h" > > /* PARF registers */ > #define PARF_SYS_CTRL 0x00 > @@ -137,9 +138,6 @@ > #define CORE_RESET_TIME_US_MAX 1005 > #define WAKE_DELAY_US 2000 /* 2 ms */ > > -#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \ > - Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed])) > - > #define to_pcie_ep(x) dev_get_drvdata((x)->dev) > > enum qcom_pcie_ep_link_status { > @@ -278,28 +276,6 @@ static void qcom_pcie_dw_write_dbi2(struct dw_pcie *pci, void __iomem *base, > writel(0, pcie_ep->elbi + ELBI_CS2_ENABLE); > } > > -static void qcom_pcie_ep_icc_update(struct qcom_pcie_ep *pcie_ep) > -{ > - struct dw_pcie *pci = &pcie_ep->pci; > - u32 offset, status; > - int speed, width; > - int ret; > - > - if (!pcie_ep->icc_mem) > - return; > - > - offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); > - status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA); > - > - speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status); > - width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status); > - > - ret = icc_set_bw(pcie_ep->icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed)); > - if (ret) > - dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", > - ret); > -} > - > static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep) > { > struct dw_pcie *pci = &pcie_ep->pci; > @@ -325,14 +301,7 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep) > if (ret) > goto err_phy_exit; > > - /* > - * Some Qualcomm platforms require interconnect bandwidth constraints > - * to be set before enabling interconnect clocks. > - * > - * Set an initial peak bandwidth corresponding to single-lane Gen 1 > - * for the pcie-mem path. > - */ > - ret = icc_set_bw(pcie_ep->icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1)); > + ret = qcom_pcie_cmn_icc_init(pci, pcie_ep->icc_mem); > if (ret) { > dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", > ret); > @@ -616,7 +585,7 @@ static int qcom_pcie_ep_get_resources(struct platform_device *pdev, > if (IS_ERR(pcie_ep->phy)) > ret = PTR_ERR(pcie_ep->phy); > > - pcie_ep->icc_mem = devm_of_icc_get(dev, "pcie-mem"); > + ret = qcom_pcie_cmn_icc_get_resource(&pcie_ep->pci, pcie_ep->icc_mem); > if (IS_ERR(pcie_ep->icc_mem)) > ret = PTR_ERR(pcie_ep->icc_mem); > > @@ -643,7 +612,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data) > } else if (FIELD_GET(PARF_INT_ALL_BME, status)) { > dev_dbg(dev, "Received BME event. Link is enabled!\n"); > pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED; > - qcom_pcie_ep_icc_update(pcie_ep); > + qcom_pcie_cmn_icc_update(pci, pcie_ep->icc_mem); > pci_epc_bme_notify(pci->ep.epc); > } else if (FIELD_GET(PARF_INT_ALL_PM_TURNOFF, status)) { > dev_dbg(dev, "Received PM Turn-off event! Entering L23\n"); > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 2ce2a3bd932b..57a08294c561 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -32,6 +32,7 @@ > #include <linux/types.h> > > #include "../../pci.h" > +#include "pcie-qcom-cmn.h" > #include "pcie-designware.h" > > /* PARF registers */ > @@ -147,9 +148,6 @@ > > #define QCOM_PCIE_CRC8_POLYNOMIAL (BIT(2) | BIT(1) | BIT(0)) > > -#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \ > - Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed])) > - > #define QCOM_PCIE_1_0_0_MAX_CLOCKS 4 > struct qcom_pcie_resources_1_0_0 { > struct clk_bulk_data clks[QCOM_PCIE_1_0_0_MAX_CLOCKS]; > @@ -1363,59 +1361,6 @@ static const struct dw_pcie_ops dw_pcie_ops = { > .start_link = qcom_pcie_start_link, > }; > > -static int qcom_pcie_icc_init(struct qcom_pcie *pcie) > -{ > - struct dw_pcie *pci = pcie->pci; > - int ret; > - > - pcie->icc_mem = devm_of_icc_get(pci->dev, "pcie-mem"); > - if (IS_ERR(pcie->icc_mem)) > - return PTR_ERR(pcie->icc_mem); > - > - /* > - * Some Qualcomm platforms require interconnect bandwidth constraints > - * to be set before enabling interconnect clocks. > - * > - * Set an initial peak bandwidth corresponding to single-lane Gen 1 > - * for the pcie-mem path. > - */ > - ret = icc_set_bw(pcie->icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1)); > - if (ret) { > - dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", > - ret); > - return ret; > - } > - > - return 0; > -} > - > -static void qcom_pcie_icc_update(struct qcom_pcie *pcie) > -{ > - struct dw_pcie *pci = pcie->pci; > - u32 offset, status; > - int speed, width; > - int ret; > - > - if (!pcie->icc_mem) > - return; > - > - offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); > - status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA); > - > - /* Only update constraints if link is up. */ > - if (!(status & PCI_EXP_LNKSTA_DLLLA)) > - return; > - > - speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status); > - width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status); > - > - ret = icc_set_bw(pcie->icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed)); > - if (ret) { > - dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", > - ret); > - } > -} > - > static int qcom_pcie_link_transition_count(struct seq_file *s, void *data) > { > struct qcom_pcie *pcie = (struct qcom_pcie *)dev_get_drvdata(s->private); > @@ -1524,7 +1469,11 @@ static int qcom_pcie_probe(struct platform_device *pdev) > goto err_pm_runtime_put; > } > > - ret = qcom_pcie_icc_init(pcie); > + ret = qcom_pcie_cmn_icc_get_resource(pcie->pci, pcie->icc_mem); > + if (ret) > + goto err_pm_runtime_put; > + > + ret = qcom_pcie_cmn_icc_init(pcie->pci, pcie->icc_mem); > if (ret) > goto err_pm_runtime_put; > > @@ -1546,7 +1495,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) > goto err_phy_exit; > } > > - qcom_pcie_icc_update(pcie); > + qcom_pcie_cmn_icc_update(pcie->pci, pcie->icc_mem); > > if (pcie->mhi) > qcom_pcie_init_debugfs(pcie); > @@ -1613,7 +1562,7 @@ static int qcom_pcie_resume_noirq(struct device *dev) > pcie->suspended = false; > } > > - qcom_pcie_icc_update(pcie); > + qcom_pcie_cmn_icc_update(pcie->pci, pcie->icc_mem); > > return 0; > } > -- > 2.43.2 >
On Thu, Feb 29, 2024 at 09:11:34PM -0800, Shashank Babu Chinta Venkata wrote: > Refactor common code from RC(Root Complex) and EP(End Point) > drivers and move them to a common repository. This acts as placeholder > for common source code for both drivers avoiding duplication. > > Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com> > --- > drivers/pci/controller/dwc/Kconfig | 5 ++ > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-qcom-cmn.c | 85 ++++++++++++++++++++++ > drivers/pci/controller/dwc/pcie-qcom-cmn.h | 30 ++++++++ > drivers/pci/controller/dwc/pcie-qcom-ep.c | 39 +--------- > drivers/pci/controller/dwc/pcie-qcom.c | 67 ++--------------- > 6 files changed, 133 insertions(+), 94 deletions(-) > create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.c > create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.h Hmm. I'm a little ambivalent about adding two new files. Overall I think I prefer the drivers that include both RC and EP mode in a single source file because one file is easier to browse than four and more things can be static. A single file would also reduce quite a bit more duplication between pcie-qcom.c and pcie-qcom-ep.c, e.g., register names and fields with needlessly different names: #define AUX_PWR_DET BIT(4) # pcie-qcom.c #define PARF_SYS_CTRL_AUX_PWR_DET BIT(4) # pcie-qcom-ep.c I do see PCIE_QCOM is bool and PCIE_QCOM_EP is tristate, so that and other considerations might make a single source file impractical. > +++ b/drivers/pci/controller/dwc/Makefile > @@ -27,6 +27,7 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o > obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o > obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o > obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o > +obj-$(CONFIG_PCIE_QCOM_CMN) += pcie-qcom-cmn.o If we have to have pcie-qcom-cmn.o, at least move this next to the existing lines: obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o > +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c > @@ -0,0 +1,85 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved. > + * Copyright 2015, 2021 Linaro Limited. > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > + * Spurious blank line. > +int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem) I don't see the value of adding "cmn" in the middle of the names. > +{ > + int ret = 0; > + > + if (IS_ERR(pci)) > + return PTR_ERR(pci); > + > + icc_mem = devm_of_icc_get(pci->dev, "pcie-mem"); > + if (IS_ERR(icc_mem)) > + return PTR_ERR(icc_mem); > + > + return ret; No need for the "ret" variable since it's never assigned. "return 0" here would be easier to read. > +int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem) > +{ > + int ret = 0; Unnecessary initialization. > +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h > @@ -0,0 +1,30 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved. > + * Copyright 2015, 2021 Linaro Limited. > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/pci.h> > +#include "../../pci.h" > +#include "pcie-designware.h" > + > +#ifdef CONFIG_PCIE_QCOM_CMN Why the #ifdef wrapper? And why do we need the stubs when CONFIG_PCIE_QCOM_CMN isn't defined? > +#else > +static inline int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem) > +{ > + return 0; > +}
On 1.03.2024 06:11, Shashank Babu Chinta Venkata wrote: > Refactor common code from RC(Root Complex) and EP(End Point) > drivers and move them to a common repository. This acts as placeholder > for common source code for both drivers avoiding duplication. > > Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com> > --- This "conveniently" conflicts with your colleague's patches.. https://lore.kernel.org/linux-arm-msm/20240223-opp_support-v7-3-10b4363d7e71@quicinc.com/ Konrad
On Fri, Mar 01, 2024 at 01:44:56PM -0600, Bjorn Helgaas wrote: > On Thu, Feb 29, 2024 at 09:11:34PM -0800, Shashank Babu Chinta Venkata wrote: > > Refactor common code from RC(Root Complex) and EP(End Point) > > drivers and move them to a common repository. This acts as placeholder > > for common source code for both drivers avoiding duplication. > > > > Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com> > > --- > > drivers/pci/controller/dwc/Kconfig | 5 ++ > > drivers/pci/controller/dwc/Makefile | 1 + > > drivers/pci/controller/dwc/pcie-qcom-cmn.c | 85 ++++++++++++++++++++++ > > drivers/pci/controller/dwc/pcie-qcom-cmn.h | 30 ++++++++ > > drivers/pci/controller/dwc/pcie-qcom-ep.c | 39 +--------- > > drivers/pci/controller/dwc/pcie-qcom.c | 67 ++--------------- > > 6 files changed, 133 insertions(+), 94 deletions(-) > > create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.c > > create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.h > > Hmm. I'm a little ambivalent about adding two new files. Overall I > think I prefer the drivers that include both RC and EP mode in a > single source file because one file is easier to browse than four and > more things can be static. > > A single file would also reduce quite a bit more duplication between > pcie-qcom.c and pcie-qcom-ep.c, e.g., register names and fields with > needlessly different names: > > #define AUX_PWR_DET BIT(4) # pcie-qcom.c > #define PARF_SYS_CTRL_AUX_PWR_DET BIT(4) # pcie-qcom-ep.c > > I do see PCIE_QCOM is bool and PCIE_QCOM_EP is tristate, so that and > other considerations might make a single source file impractical. > Yeah, we explored that option while adding the EP driver and it didn't look feasible. - Mani > > +++ b/drivers/pci/controller/dwc/Makefile > > @@ -27,6 +27,7 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o > > obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o > > obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o > > obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o > > +obj-$(CONFIG_PCIE_QCOM_CMN) += pcie-qcom-cmn.o > > If we have to have pcie-qcom-cmn.o, at least move this next to the > existing lines: > > obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o > obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o > > > +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c > > @@ -0,0 +1,85 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved. > > + * Copyright 2015, 2021 Linaro Limited. > > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > > + * > > Spurious blank line. > > > +int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem) > > I don't see the value of adding "cmn" in the middle of the names. > > > +{ > > + int ret = 0; > > + > > + if (IS_ERR(pci)) > > + return PTR_ERR(pci); > > + > > + icc_mem = devm_of_icc_get(pci->dev, "pcie-mem"); > > + if (IS_ERR(icc_mem)) > > + return PTR_ERR(icc_mem); > > + > > + return ret; > > No need for the "ret" variable since it's never assigned. "return 0" > here would be easier to read. > > > +int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem) > > +{ > > + int ret = 0; > > Unnecessary initialization. > > > +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h > > @@ -0,0 +1,30 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved. > > + * Copyright 2015, 2021 Linaro Limited. > > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. > > + */ > > + > > +#include <linux/pci.h> > > +#include "../../pci.h" > > +#include "pcie-designware.h" > > + > > +#ifdef CONFIG_PCIE_QCOM_CMN > > Why the #ifdef wrapper? And why do we need the stubs when > CONFIG_PCIE_QCOM_CMN isn't defined? > > > +#else > > +static inline int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem) > > +{ > > + return 0; > > +}
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig index 8afacc90c63b..41d2746edc5f 100644 --- a/drivers/pci/controller/dwc/Kconfig +++ b/drivers/pci/controller/dwc/Kconfig @@ -265,12 +265,16 @@ config PCIE_DW_PLAT_EP order to enable device-specific features PCI_DW_PLAT_EP must be selected. +config PCIE_QCOM_CMN + bool + config PCIE_QCOM bool "Qualcomm PCIe controller (host mode)" depends on OF && (ARCH_QCOM || COMPILE_TEST) depends on PCI_MSI select PCIE_DW_HOST select CRC8 + select PCIE_QCOM_CMN help Say Y here to enable PCIe controller support on Qualcomm SoCs. The PCIe controller uses the DesignWare core plus Qualcomm-specific @@ -281,6 +285,7 @@ config PCIE_QCOM_EP depends on OF && (ARCH_QCOM || COMPILE_TEST) depends on PCI_ENDPOINT select PCIE_DW_EP + select PCIE_QCOM_CMN help Say Y here to enable support for the PCIe controllers on Qualcomm SoCs to work in endpoint mode. The PCIe controller uses the DesignWare core diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile index bac103faa523..022dc73c38a5 100644 --- a/drivers/pci/controller/dwc/Makefile +++ b/drivers/pci/controller/dwc/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o +obj-$(CONFIG_PCIE_QCOM_CMN) += pcie-qcom-cmn.o # The following drivers are for devices that use the generic ACPI # pci_root.c driver but don't support standard ECAM config access. diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.c b/drivers/pci/controller/dwc/pcie-qcom-cmn.c new file mode 100644 index 000000000000..0f8d004fbc79 --- /dev/null +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved. + * Copyright 2015, 2021 Linaro Limited. + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. + * + */ + +#include <linux/debugfs.h> +#include <linux/pci.h> +#include <linux/interconnect.h> + +#include "../../pci.h" +#include "pcie-designware.h" +#include "pcie-qcom-cmn.h" + + +#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \ + Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed])) + + +int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem) +{ + int ret = 0; + + if (IS_ERR(pci)) + return PTR_ERR(pci); + + icc_mem = devm_of_icc_get(pci->dev, "pcie-mem"); + if (IS_ERR(icc_mem)) + return PTR_ERR(icc_mem); + + return ret; +} +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_get_resource); + +int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem) +{ + int ret = 0; + + if (IS_ERR(pci)) + return PTR_ERR(pci); + + if (IS_ERR(icc_mem)) + return PTR_ERR(icc_mem); + + /* + * Some Qualcomm platforms require interconnect bandwidth constraints + * to be set before enabling interconnect clocks. + * + * Set an initial peak bandwidth corresponding to single-lane Gen 1 + * for the pcie-mem path. + */ + ret = icc_set_bw(icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1)); + if (ret) { + dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", + ret); + return ret; + } + + return 0; +} +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_init); + +void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem) +{ + u32 offset, status; + int speed, width; + int ret; + + if (!icc_mem) + return; + + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); + status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA); + + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status); + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status); + + ret = icc_set_bw(icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed)); + if (ret) + dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", + ret); +} +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_update); diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.h b/drivers/pci/controller/dwc/pcie-qcom-cmn.h new file mode 100644 index 000000000000..8794dbd4775c --- /dev/null +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved. + * Copyright 2015, 2021 Linaro Limited. + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#include <linux/pci.h> +#include "../../pci.h" +#include "pcie-designware.h" + +#ifdef CONFIG_PCIE_QCOM_CMN +int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem); +int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem); +void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem); +#else +static inline int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem) +{ + return 0; +} + +static inline int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem) +{ + return 0; +} + +static inline void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem) +{ +} +#endif diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c index 36e5e80cd22f..ce6343426de8 100644 --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c @@ -25,6 +25,7 @@ #include "../../pci.h" #include "pcie-designware.h" +#include "pcie-qcom-cmn.h" /* PARF registers */ #define PARF_SYS_CTRL 0x00 @@ -137,9 +138,6 @@ #define CORE_RESET_TIME_US_MAX 1005 #define WAKE_DELAY_US 2000 /* 2 ms */ -#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \ - Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed])) - #define to_pcie_ep(x) dev_get_drvdata((x)->dev) enum qcom_pcie_ep_link_status { @@ -278,28 +276,6 @@ static void qcom_pcie_dw_write_dbi2(struct dw_pcie *pci, void __iomem *base, writel(0, pcie_ep->elbi + ELBI_CS2_ENABLE); } -static void qcom_pcie_ep_icc_update(struct qcom_pcie_ep *pcie_ep) -{ - struct dw_pcie *pci = &pcie_ep->pci; - u32 offset, status; - int speed, width; - int ret; - - if (!pcie_ep->icc_mem) - return; - - offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); - status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA); - - speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status); - width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status); - - ret = icc_set_bw(pcie_ep->icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed)); - if (ret) - dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", - ret); -} - static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep) { struct dw_pcie *pci = &pcie_ep->pci; @@ -325,14 +301,7 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep) if (ret) goto err_phy_exit; - /* - * Some Qualcomm platforms require interconnect bandwidth constraints - * to be set before enabling interconnect clocks. - * - * Set an initial peak bandwidth corresponding to single-lane Gen 1 - * for the pcie-mem path. - */ - ret = icc_set_bw(pcie_ep->icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1)); + ret = qcom_pcie_cmn_icc_init(pci, pcie_ep->icc_mem); if (ret) { dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", ret); @@ -616,7 +585,7 @@ static int qcom_pcie_ep_get_resources(struct platform_device *pdev, if (IS_ERR(pcie_ep->phy)) ret = PTR_ERR(pcie_ep->phy); - pcie_ep->icc_mem = devm_of_icc_get(dev, "pcie-mem"); + ret = qcom_pcie_cmn_icc_get_resource(&pcie_ep->pci, pcie_ep->icc_mem); if (IS_ERR(pcie_ep->icc_mem)) ret = PTR_ERR(pcie_ep->icc_mem); @@ -643,7 +612,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data) } else if (FIELD_GET(PARF_INT_ALL_BME, status)) { dev_dbg(dev, "Received BME event. Link is enabled!\n"); pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED; - qcom_pcie_ep_icc_update(pcie_ep); + qcom_pcie_cmn_icc_update(pci, pcie_ep->icc_mem); pci_epc_bme_notify(pci->ep.epc); } else if (FIELD_GET(PARF_INT_ALL_PM_TURNOFF, status)) { dev_dbg(dev, "Received PM Turn-off event! Entering L23\n"); diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 2ce2a3bd932b..57a08294c561 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -32,6 +32,7 @@ #include <linux/types.h> #include "../../pci.h" +#include "pcie-qcom-cmn.h" #include "pcie-designware.h" /* PARF registers */ @@ -147,9 +148,6 @@ #define QCOM_PCIE_CRC8_POLYNOMIAL (BIT(2) | BIT(1) | BIT(0)) -#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \ - Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed])) - #define QCOM_PCIE_1_0_0_MAX_CLOCKS 4 struct qcom_pcie_resources_1_0_0 { struct clk_bulk_data clks[QCOM_PCIE_1_0_0_MAX_CLOCKS]; @@ -1363,59 +1361,6 @@ static const struct dw_pcie_ops dw_pcie_ops = { .start_link = qcom_pcie_start_link, }; -static int qcom_pcie_icc_init(struct qcom_pcie *pcie) -{ - struct dw_pcie *pci = pcie->pci; - int ret; - - pcie->icc_mem = devm_of_icc_get(pci->dev, "pcie-mem"); - if (IS_ERR(pcie->icc_mem)) - return PTR_ERR(pcie->icc_mem); - - /* - * Some Qualcomm platforms require interconnect bandwidth constraints - * to be set before enabling interconnect clocks. - * - * Set an initial peak bandwidth corresponding to single-lane Gen 1 - * for the pcie-mem path. - */ - ret = icc_set_bw(pcie->icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1)); - if (ret) { - dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", - ret); - return ret; - } - - return 0; -} - -static void qcom_pcie_icc_update(struct qcom_pcie *pcie) -{ - struct dw_pcie *pci = pcie->pci; - u32 offset, status; - int speed, width; - int ret; - - if (!pcie->icc_mem) - return; - - offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); - status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA); - - /* Only update constraints if link is up. */ - if (!(status & PCI_EXP_LNKSTA_DLLLA)) - return; - - speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status); - width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status); - - ret = icc_set_bw(pcie->icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed)); - if (ret) { - dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n", - ret); - } -} - static int qcom_pcie_link_transition_count(struct seq_file *s, void *data) { struct qcom_pcie *pcie = (struct qcom_pcie *)dev_get_drvdata(s->private); @@ -1524,7 +1469,11 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_pm_runtime_put; } - ret = qcom_pcie_icc_init(pcie); + ret = qcom_pcie_cmn_icc_get_resource(pcie->pci, pcie->icc_mem); + if (ret) + goto err_pm_runtime_put; + + ret = qcom_pcie_cmn_icc_init(pcie->pci, pcie->icc_mem); if (ret) goto err_pm_runtime_put; @@ -1546,7 +1495,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) goto err_phy_exit; } - qcom_pcie_icc_update(pcie); + qcom_pcie_cmn_icc_update(pcie->pci, pcie->icc_mem); if (pcie->mhi) qcom_pcie_init_debugfs(pcie); @@ -1613,7 +1562,7 @@ static int qcom_pcie_resume_noirq(struct device *dev) pcie->suspended = false; } - qcom_pcie_icc_update(pcie); + qcom_pcie_cmn_icc_update(pcie->pci, pcie->icc_mem); return 0; }
Refactor common code from RC(Root Complex) and EP(End Point) drivers and move them to a common repository. This acts as placeholder for common source code for both drivers avoiding duplication. Signed-off-by: Shashank Babu Chinta Venkata <quic_schintav@quicinc.com> --- drivers/pci/controller/dwc/Kconfig | 5 ++ drivers/pci/controller/dwc/Makefile | 1 + drivers/pci/controller/dwc/pcie-qcom-cmn.c | 85 ++++++++++++++++++++++ drivers/pci/controller/dwc/pcie-qcom-cmn.h | 30 ++++++++ drivers/pci/controller/dwc/pcie-qcom-ep.c | 39 +--------- drivers/pci/controller/dwc/pcie-qcom.c | 67 ++--------------- 6 files changed, 133 insertions(+), 94 deletions(-) create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.c create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.h