* [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex
@ 2024-11-06 22:13 Mayank Rana
2024-11-06 22:13 ` [PATCH v3 1/4] PCI: dwc: Export dwc MSI controller related APIs Mayank Rana
` (4 more replies)
0 siblings, 5 replies; 24+ messages in thread
From: Mayank Rana @ 2024-11-06 22:13 UTC (permalink / raw)
To: jingoohan1, manivannan.sadhasivam, will, lpieralisi, kw, robh,
bhelgaas, krzk
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai, Mayank Rana
Based on received feedback, this patch series adds support with existing
Linux qcom-pcie.c driver to get PCIe host root complex functionality on
Qualcomm SA8255P auto platform.
1. Interface to allow requesting firmware to manage system resources and
performing PCIe Link up (devicetree binding in terms of power domain and
runtime PM APIs is used in driver)
2. SA8255P is using Synopsys Designware PCIe controller which supports MSI
controller. Using existing MSI controller based functionality by exporting
important pcie dwc core driver based MSI APIs, and using those from
pcie-qcom.c driver.
Below architecture is used on Qualcomm SA8255P auto platform to get ECAM
compliant PCIe controller based functionality. Here firmware VM based PCIe
driver takes care of resource management and performing PCIe link related
handling (D0 and D3cold). Linux pcie-qcom.c driver uses power domain to
request firmware VM to perform these operations using SCMI interface.
--------------------
┌────────────────────────┐
│ │
┌──────────────────────┐ │ SHARED MEMORY │ ┌──────────────────────────┐
│ Firmware VM │ │ │ │ Linux VM │
│ ┌─────────┐ │ │ │ │ ┌────────────────┐ │
│ │ Drivers │ ┌──────┐ │ │ │ │ │ PCIE Qcom │ │
│ │ PCIE PHY◄─┤ │ │ │ ┌────────────────┐ │ │ │ driver │ │
│ │ │ │ SCMI │ │ │ │ │ │ │ │ │ │
│ │PCIE CTL │ │ │ ├─────────┼───► PCIE ◄───┼─────┐ │ └──┬──────────▲──┘ │
│ │ ├─►Server│ │ │ │ SHMEM │ │ │ │ │ │ │
│ │Clk, Vreg│ │ │ │ │ │ │ │ │ │ ┌──▼──────────┴──┐ │
│ │GPIO,GDSC│ └─▲──┬─┘ │ │ └────────────────┘ │ └──────┼────┤PCIE SCMI Inst │ │
│ └─────────┘ │ │ │ │ │ │ └──▲──────────┬──┘ │
│ │ │ │ │ │ │ │ │ │
└───────────────┼──┼───┘ │ │ └───────┼──────────┼───────┘
│ │ │ │ │ │
│ │ └────────────────────────┘ │ │
│ │ │ │
│ │ │ │
│ │ │ │
│ │ │IRQ │HVC
IRQ │ │HVC │ │
│ │ │ │
│ │ │ │
│ │ │ │
┌─────────────────┴──▼───────────────────────────────────────────────────────────┴──────────▼──────────────┐
│ │
│ │
│ HYPERVISOR │
│ │
│ │
│ │
└──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
┌─────────────┐ ┌─────────────┐ ┌──────────┐ ┌───────────┐ ┌─────────────┐ ┌────────────┐
│ │ │ │ │ │ │ │ │ PCIE │ │ PCIE │
│ CLOCK │ │ REGULATOR │ │ GPIO │ │ GDSC │ │ PHY │ │ controller │
└─────────────┘ └─────────────┘ └──────────┘ └───────────┘ └─────────────┘ └────────────┘
----------
Changes in V3:
- Drop usage of PCIE host generic driver usage, and splitting of MSI functionality
- Modified existing pcie-qcom.c driver to add support for getting ECAM compliant and firmware managed
PCIe root complex functionality
Link to v2: https://lore.kernel.org/linux-arm-kernel/925d1eca-975f-4eec-bdf8-ca07a892361a@quicinc.com/T/
Changes in V2:
- Drop new PCIe Qcom ECAM driver, and use existing PCIe designware based MSI functionality
- Add power domain based functionality within existing ECAM driver
Link to v1: https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
Tested:
- Validated NVME functionality with PCIe0 on SA8255P-RIDE platform
Mayank Rana (3):
PCI: dwc: Export dwc MSI controller related APIs
PCI: qcom: Add firmware managed ECAM compliant PCIe root complex
functionality
dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root
complex
.../devicetree/bindings/pci/qcom,pcie-sa8255p.yaml | 100 +++++++++++++++++++++
drivers/pci/controller/dwc/Kconfig | 1 +
drivers/pci/controller/dwc/pcie-designware-host.c | 38 ++++----
drivers/pci/controller/dwc/pcie-designware.h | 14 +++
drivers/pci/controller/dwc/pcie-qcom.c | 69 ++++++++++++--
5 files changed, 199 insertions(+), 23 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 1/4] PCI: dwc: Export dwc MSI controller related APIs
2024-11-06 22:13 [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Mayank Rana
@ 2024-11-06 22:13 ` Mayank Rana
2024-11-15 9:14 ` Manivannan Sadhasivam
2024-11-06 22:13 ` [PATCH v3 2/4] PCI: host-generic: Export gen_pci_init() API to allow ECAM creation Mayank Rana
` (3 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Mayank Rana @ 2024-11-06 22:13 UTC (permalink / raw)
To: jingoohan1, manivannan.sadhasivam, will, lpieralisi, kw, robh,
bhelgaas, krzk
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai, Mayank Rana
To allow dwc PCIe controller based MSI functionality from ECAM pcie
driver, export dw_pcie_msi_host_init(), dw_pcie_msi_init() and
dw_pcie_msi_free() APIs. Also move MSI IRQ related initialization code
into dw_pcie_msi_init() as this code executes before dw_pcie_msi_init()
API to use with ECAM driver.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
.../pci/controller/dwc/pcie-designware-host.c | 38 ++++++++++---------
drivers/pci/controller/dwc/pcie-designware.h | 14 +++++++
2 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 3e41865c7290..25020a090db8 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -250,7 +250,7 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
return 0;
}
-static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
+void dw_pcie_free_msi(struct dw_pcie_rp *pp)
{
u32 ctrl;
@@ -263,19 +263,34 @@ static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
irq_domain_remove(pp->msi_domain);
irq_domain_remove(pp->irq_domain);
}
+EXPORT_SYMBOL_GPL(dw_pcie_free_msi);
-static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
+void dw_pcie_msi_init(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
u64 msi_target = (u64)pp->msi_data;
+ u32 ctrl, num_ctrls;
if (!pci_msi_enabled() || !pp->has_msi_ctrl)
return;
+ num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+
+ /* Initialize IRQ Status array */
+ for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
+ dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
+ (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
+ pp->irq_mask[ctrl]);
+ dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
+ (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
+ ~0);
+ }
+
/* Program the msi_data */
dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
}
+EXPORT_SYMBOL_GPL(dw_pcie_msi_init);
static int dw_pcie_parse_split_msi_irq(struct dw_pcie_rp *pp)
{
@@ -317,7 +332,7 @@ static int dw_pcie_parse_split_msi_irq(struct dw_pcie_rp *pp)
return 0;
}
-static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
+int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct device *dev = pci->dev;
@@ -391,6 +406,7 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
return 0;
}
+EXPORT_SYMBOL_GPL(dw_pcie_msi_host_init);
static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
{
@@ -802,7 +818,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- u32 val, ctrl, num_ctrls;
+ u32 val;
int ret;
/*
@@ -813,20 +829,6 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
dw_pcie_setup(pci);
- if (pp->has_msi_ctrl) {
- num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
-
- /* Initialize IRQ Status array */
- for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
- (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
- pp->irq_mask[ctrl]);
- dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
- (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
- ~0);
- }
- }
-
dw_pcie_msi_init(pp);
/* Setup RC BARs */
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 347ab74ac35a..ef748d82c663 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -679,6 +679,9 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
#ifdef CONFIG_PCIE_DW_HOST
irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
+void dw_pcie_msi_init(struct dw_pcie_rp *pp);
+int dw_pcie_msi_host_init(struct dw_pcie_rp *pp);
+void dw_pcie_free_msi(struct dw_pcie_rp *pp);
int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
int dw_pcie_host_init(struct dw_pcie_rp *pp);
void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
@@ -691,6 +694,17 @@ static inline irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp)
return IRQ_NONE;
}
+static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
+{ }
+
+static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
+{
+ return -ENODEV;
+}
+
+static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
+{ }
+
static inline int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
{
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 2/4] PCI: host-generic: Export gen_pci_init() API to allow ECAM creation
2024-11-06 22:13 [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Mayank Rana
2024-11-06 22:13 ` [PATCH v3 1/4] PCI: dwc: Export dwc MSI controller related APIs Mayank Rana
@ 2024-11-06 22:13 ` Mayank Rana
2024-11-15 9:17 ` Manivannan Sadhasivam
2024-11-06 22:13 ` [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex Mayank Rana
` (2 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Mayank Rana @ 2024-11-06 22:13 UTC (permalink / raw)
To: jingoohan1, manivannan.sadhasivam, will, lpieralisi, kw, robh,
bhelgaas, krzk
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai, Mayank Rana
Export gen_pci_init() API to create ECAM and initialized ECAM OPs
from PCIe driver which don't have way to populate driver_data as
just ECAM ops.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
drivers/pci/controller/pci-host-common.c | 3 ++-
include/linux/pci-ecam.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index cf5f59a745b3..b9460a4c5b7e 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -20,7 +20,7 @@ static void gen_pci_unmap_cfg(void *ptr)
pci_ecam_free((struct pci_config_window *)ptr);
}
-static struct pci_config_window *gen_pci_init(struct device *dev,
+struct pci_config_window *gen_pci_init(struct device *dev,
struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops)
{
int err;
@@ -48,6 +48,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
return cfg;
}
+EXPORT_SYMBOL_GPL(gen_pci_init);
int pci_host_common_probe(struct platform_device *pdev)
{
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index 3a4860bd2758..386c08349169 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -94,5 +94,7 @@ extern const struct pci_ecam_ops loongson_pci_ecam_ops; /* Loongson PCIe */
/* for DT-based PCI controllers that support ECAM */
int pci_host_common_probe(struct platform_device *pdev);
void pci_host_common_remove(struct platform_device *pdev);
+struct pci_config_window *gen_pci_init(struct device *dev,
+ struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
#endif
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex
2024-11-06 22:13 [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Mayank Rana
2024-11-06 22:13 ` [PATCH v3 1/4] PCI: dwc: Export dwc MSI controller related APIs Mayank Rana
2024-11-06 22:13 ` [PATCH v3 2/4] PCI: host-generic: Export gen_pci_init() API to allow ECAM creation Mayank Rana
@ 2024-11-06 22:13 ` Mayank Rana
2024-11-07 9:35 ` Krzysztof Kozlowski
2024-11-15 10:55 ` Manivannan Sadhasivam
2024-11-06 22:13 ` [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality Mayank Rana
2024-11-15 11:28 ` [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Manivannan Sadhasivam
4 siblings, 2 replies; 24+ messages in thread
From: Mayank Rana @ 2024-11-06 22:13 UTC (permalink / raw)
To: jingoohan1, manivannan.sadhasivam, will, lpieralisi, kw, robh,
bhelgaas, krzk
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai, Mayank Rana
On SA8255p, PCIe root complex is managed by firmware using power-domain
based handling. This root complex is configured as ECAM compliant.
Document required configuration to enable PCIe root complex.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
.../bindings/pci/qcom,pcie-sa8255p.yaml | 103 ++++++++++++++++++
1 file changed, 103 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
new file mode 100644
index 000000000000..9b09c3923ba0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/qcom,pcie-sa8255p.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SA8255p based firmware managed and ECAM compliant PCIe Root Complex
+
+maintainers:
+ - Bjorn Andersson <andersson@kernel.org>
+ - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
+
+description:
+ Qualcomm SA8255p SoC PCIe root complex controller is based on the Synopsys
+ DesignWare PCIe IP which is managed by firmware, and configured in ECAM mode.
+
+properties:
+ compatible:
+ const: qcom,pcie-sa8255p
+
+ reg:
+ description:
+ The Configuration Space base address and size, as accessed from the parent
+ bus. The base address corresponds to the first bus in the "bus-range"
+ property. If no "bus-range" is specified, this will be bus 0 (the
+ default).
+ maxItems: 1
+
+ ranges:
+ description:
+ As described in IEEE Std 1275-1994, but must provide at least a
+ definition of non-prefetchable memory. One or both of prefetchable Memory
+ may also be provided.
+ minItems: 1
+ maxItems: 2
+
+ interrupts:
+ minItems: 8
+ maxItems: 8
+
+ interrupt-names:
+ items:
+ - const: msi0
+ - const: msi1
+ - const: msi2
+ - const: msi3
+ - const: msi4
+ - const: msi5
+ - const: msi6
+ - const: msi7
+
+ power-domains:
+ maxItems: 1
+
+ dma-coherent: true
+ iommu-map: true
+
+required:
+ - compatible
+ - reg
+ - ranges
+ - power-domains
+
+allOf:
+ - $ref: /schemas/pci/pci-host-bridge.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pci@1c00000 {
+ compatible = "qcom,pcie-sa8255p";
+ reg = <0x4 0x00000000 0 0x10000000>;
+ device_type = "pci";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges = <0x02000000 0x0 0x40100000 0x0 0x40100000 0x0 0x1ff00000>,
+ <0x43000000 0x4 0x10100000 0x4 0x10100000 0x0 0x40000000>;
+ bus-range = <0x00 0xff>;
+ dma-coherent;
+ linux,pci-domain = <0>;
+ power-domains = <&scmi5_pd 0>;
+ iommu-map = <0x0 &pcie_smmu 0x0000 0x1>,
+ <0x100 &pcie_smmu 0x0001 0x1>;
+ interrupt-parent = <&intc>;
+ interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi0", "msi1", "msi2", "msi3",
+ "msi4", "msi5", "msi6", "msi7";
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-06 22:13 [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Mayank Rana
` (2 preceding siblings ...)
2024-11-06 22:13 ` [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex Mayank Rana
@ 2024-11-06 22:13 ` Mayank Rana
2024-11-07 8:45 ` neil.armstrong
` (2 more replies)
2024-11-15 11:28 ` [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Manivannan Sadhasivam
4 siblings, 3 replies; 24+ messages in thread
From: Mayank Rana @ 2024-11-06 22:13 UTC (permalink / raw)
To: jingoohan1, manivannan.sadhasivam, will, lpieralisi, kw, robh,
bhelgaas, krzk
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai, Mayank Rana
On SA8255p ride platform, PCIe root complex is firmware managed as well
configured into ECAM compliant mode. This change adds functionality to
enable resource management (system resource as well PCIe controller and
PHY configuration) through firmware, and enumerating ECAM compliant root
complex.
Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
---
drivers/pci/controller/dwc/Kconfig | 1 +
drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
2 files changed, 108 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index b6d6778b0698..0fe76bd39d69 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -275,6 +275,7 @@ config PCIE_QCOM
select PCIE_DW_HOST
select CRC8
select PCIE_QCOM_COMMON
+ select PCI_HOST_COMMON
help
Say Y here to enable PCIe controller support on Qualcomm SoCs. The
PCIe controller uses the DesignWare core plus Qualcomm-specific
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index ef44a82be058..2cb74f902baf 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -21,7 +21,9 @@
#include <linux/limits.h>
#include <linux/init.h>
#include <linux/of.h>
+#include <linux/of_pci.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/pm_opp.h>
#include <linux/pm_runtime.h>
#include <linux/platform_device.h>
@@ -254,10 +256,12 @@ struct qcom_pcie_ops {
* @ops: qcom PCIe ops structure
* @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
* snooping
+ * @firmware_managed: Set if PCIe root complex is firmware managed
*/
struct qcom_pcie_cfg {
const struct qcom_pcie_ops *ops;
bool override_no_snoop;
+ bool firmware_managed;
bool no_l0s;
};
@@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
.no_l0s = true,
};
+static const struct qcom_pcie_cfg cfg_fw_managed = {
+ .firmware_managed = true,
+};
+
static const struct dw_pcie_ops dw_pcie_ops = {
.link_up = qcom_pcie_link_up,
.start_link = qcom_pcie_start_link,
@@ -1566,6 +1574,51 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
return IRQ_HANDLED;
}
+static void qcom_pci_free_msi(void *ptr)
+{
+ struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
+
+ if (pp && pp->has_msi_ctrl)
+ dw_pcie_free_msi(pp);
+}
+
+static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
+{
+ struct device *dev = cfg->parent;
+ struct dw_pcie_rp *pp;
+ struct dw_pcie *pci;
+ int ret;
+
+ pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
+ if (!pci)
+ return -ENOMEM;
+
+ pci->dev = dev;
+ pp = &pci->pp;
+ pci->dbi_base = cfg->win;
+ pp->num_vectors = MSI_DEF_NUM_VECTORS;
+
+ ret = dw_pcie_msi_host_init(pp);
+ if (ret)
+ return ret;
+
+ pp->has_msi_ctrl = true;
+ dw_pcie_msi_init(pp);
+
+ ret = devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
+ return ret;
+}
+
+/* ECAM ops */
+const struct pci_ecam_ops pci_qcom_ecam_ops = {
+ .init = qcom_pcie_ecam_host_init,
+ .pci_ops = {
+ .map_bus = pci_ecam_map_bus,
+ .read = pci_generic_config_read,
+ .write = pci_generic_config_write,
+ }
+};
+
static int qcom_pcie_probe(struct platform_device *pdev)
{
const struct qcom_pcie_cfg *pcie_cfg;
@@ -1580,11 +1633,52 @@ static int qcom_pcie_probe(struct platform_device *pdev)
char *name;
pcie_cfg = of_device_get_match_data(dev);
- if (!pcie_cfg || !pcie_cfg->ops) {
- dev_err(dev, "Invalid platform data\n");
+ if (!pcie_cfg) {
+ dev_err(dev, "No platform data\n");
+ return -EINVAL;
+ }
+
+ if (!pcie_cfg->firmware_managed && !pcie_cfg->ops) {
+ dev_err(dev, "No platform ops\n");
return -EINVAL;
}
+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0)
+ goto err_pm_runtime_put;
+
+ if (pcie_cfg->firmware_managed) {
+ struct pci_host_bridge *bridge;
+ struct pci_config_window *cfg;
+
+ bridge = devm_pci_alloc_host_bridge(dev, 0);
+ if (!bridge) {
+ ret = -ENOMEM;
+ goto err_pm_runtime_put;
+ }
+
+ of_pci_check_probe_only();
+ /* Parse and map our Configuration Space windows */
+ cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
+ if (IS_ERR(cfg)) {
+ ret = PTR_ERR(cfg);
+ goto err_pm_runtime_put;
+ }
+
+ bridge->sysdata = cfg;
+ bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
+ bridge->msi_domain = true;
+
+ ret = pci_host_probe(bridge);
+ if (ret) {
+ dev_err(dev, "pci_host_probe() failed:%d\n", ret);
+ goto err_pm_runtime_put;
+ }
+
+ return ret;
+ }
+
pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
if (!pcie)
return -ENOMEM;
@@ -1593,11 +1687,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
if (!pci)
return -ENOMEM;
- pm_runtime_enable(dev);
- ret = pm_runtime_get_sync(dev);
- if (ret < 0)
- goto err_pm_runtime_put;
-
pci->dev = dev;
pci->ops = &dw_pcie_ops;
pp = &pci->pp;
@@ -1739,9 +1828,13 @@ static int qcom_pcie_probe(struct platform_device *pdev)
static int qcom_pcie_suspend_noirq(struct device *dev)
{
- struct qcom_pcie *pcie = dev_get_drvdata(dev);
+ struct qcom_pcie *pcie;
int ret = 0;
+ if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
+ return 0;
+
+ pcie = dev_get_drvdata(dev);
/*
* Set minimum bandwidth required to keep data path functional during
* suspend.
@@ -1795,9 +1888,13 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
static int qcom_pcie_resume_noirq(struct device *dev)
{
- struct qcom_pcie *pcie = dev_get_drvdata(dev);
+ struct qcom_pcie *pcie;
int ret;
+ if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
+ return 0;
+
+ pcie = dev_get_drvdata(dev);
if (pm_suspend_target_state != PM_SUSPEND_MEM) {
ret = icc_enable(pcie->icc_cpu);
if (ret) {
@@ -1830,6 +1927,7 @@ static const struct of_device_id qcom_pcie_match[] = {
{ .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
+ { .compatible = "qcom,pcie-sa8255p", .data = &cfg_fw_managed },
{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
{ .compatible = "qcom,pcie-sa8775p", .data = &cfg_1_34_0},
{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-06 22:13 ` [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality Mayank Rana
@ 2024-11-07 8:45 ` neil.armstrong
2024-11-07 17:45 ` Mayank Rana
2024-11-09 23:45 ` kernel test robot
2024-11-15 11:25 ` Manivannan Sadhasivam
2 siblings, 1 reply; 24+ messages in thread
From: neil.armstrong @ 2024-11-07 8:45 UTC (permalink / raw)
To: Mayank Rana, jingoohan1, manivannan.sadhasivam, will, lpieralisi,
kw, robh, bhelgaas, krzk
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai
Hi,
On 06/11/2024 23:13, Mayank Rana wrote:
> On SA8255p ride platform, PCIe root complex is firmware managed as well
> configured into ECAM compliant mode. This change adds functionality to
> enable resource management (system resource as well PCIe controller and
> PHY configuration) through firmware, and enumerating ECAM compliant root
> complex.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 1 +
> drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
> 2 files changed, 108 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index b6d6778b0698..0fe76bd39d69 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -275,6 +275,7 @@ config PCIE_QCOM
> select PCIE_DW_HOST
> select CRC8
> select PCIE_QCOM_COMMON
> + select PCI_HOST_COMMON
> help
> Say Y here to enable PCIe controller support on Qualcomm SoCs. The
> PCIe controller uses the DesignWare core plus Qualcomm-specific
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index ef44a82be058..2cb74f902baf 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -21,7 +21,9 @@
> #include <linux/limits.h>
> #include <linux/init.h>
> #include <linux/of.h>
> +#include <linux/of_pci.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/pm_opp.h>
> #include <linux/pm_runtime.h>
> #include <linux/platform_device.h>
> @@ -254,10 +256,12 @@ struct qcom_pcie_ops {
> * @ops: qcom PCIe ops structure
> * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
> * snooping
> + * @firmware_managed: Set if PCIe root complex is firmware managed
> */
> struct qcom_pcie_cfg {
> const struct qcom_pcie_ops *ops;
> bool override_no_snoop;
> + bool firmware_managed;
> bool no_l0s;
> };
>
> @@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
> .no_l0s = true,
> };
>
> +static const struct qcom_pcie_cfg cfg_fw_managed = {
> + .firmware_managed = true,
> +};
> +
> static const struct dw_pcie_ops dw_pcie_ops = {
> .link_up = qcom_pcie_link_up,
> .start_link = qcom_pcie_start_link,
> @@ -1566,6 +1574,51 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static void qcom_pci_free_msi(void *ptr)
> +{
> + struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
> +
> + if (pp && pp->has_msi_ctrl)
> + dw_pcie_free_msi(pp);
> +}
> +
> +static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
> +{
> + struct device *dev = cfg->parent;
> + struct dw_pcie_rp *pp;
> + struct dw_pcie *pci;
> + int ret;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> + if (!pci)
> + return -ENOMEM;
> +
> + pci->dev = dev;
> + pp = &pci->pp;
> + pci->dbi_base = cfg->win;
> + pp->num_vectors = MSI_DEF_NUM_VECTORS;
> +
> + ret = dw_pcie_msi_host_init(pp);
> + if (ret)
> + return ret;
> +
> + pp->has_msi_ctrl = true;
> + dw_pcie_msi_init(pp);
> +
> + ret = devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
> + return ret;
> +}
> +
> +/* ECAM ops */
> +const struct pci_ecam_ops pci_qcom_ecam_ops = {
> + .init = qcom_pcie_ecam_host_init,
> + .pci_ops = {
> + .map_bus = pci_ecam_map_bus,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> + }
> +};
> +
> static int qcom_pcie_probe(struct platform_device *pdev)
> {
> const struct qcom_pcie_cfg *pcie_cfg;
> @@ -1580,11 +1633,52 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> char *name;
>
> pcie_cfg = of_device_get_match_data(dev);
> - if (!pcie_cfg || !pcie_cfg->ops) {
> - dev_err(dev, "Invalid platform data\n");
> + if (!pcie_cfg) {
> + dev_err(dev, "No platform data\n");
> + return -EINVAL;
> + }
> +
> + if (!pcie_cfg->firmware_managed && !pcie_cfg->ops) {
> + dev_err(dev, "No platform ops\n");
> return -EINVAL;
> }
>
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0)
> + goto err_pm_runtime_put;
> +
> + if (pcie_cfg->firmware_managed) {
> + struct pci_host_bridge *bridge;
> + struct pci_config_window *cfg;
> +
> + bridge = devm_pci_alloc_host_bridge(dev, 0);
> + if (!bridge) {
> + ret = -ENOMEM;
> + goto err_pm_runtime_put;
> + }
> +
> + of_pci_check_probe_only();
> + /* Parse and map our Configuration Space windows */
> + cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
> + if (IS_ERR(cfg)) {
> + ret = PTR_ERR(cfg);
> + goto err_pm_runtime_put;
> + }
> +
> + bridge->sysdata = cfg;
> + bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
> + bridge->msi_domain = true;
> +
> + ret = pci_host_probe(bridge);
> + if (ret) {
> + dev_err(dev, "pci_host_probe() failed:%d\n", ret);
> + goto err_pm_runtime_put;
> + }
> +
> + return ret;
> + }
> +
> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> if (!pcie)
> return -ENOMEM;
> @@ -1593,11 +1687,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> if (!pci)
> return -ENOMEM;
>
> - pm_runtime_enable(dev);
> - ret = pm_runtime_get_sync(dev);
> - if (ret < 0)
> - goto err_pm_runtime_put;
> -
> pci->dev = dev;
> pci->ops = &dw_pcie_ops;
> pp = &pci->pp;
> @@ -1739,9 +1828,13 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>
> static int qcom_pcie_suspend_noirq(struct device *dev)
> {
> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
> + struct qcom_pcie *pcie;
> int ret = 0;
>
> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
Can't you use if (pcie_cfg->firmware_managed) here instead ?
> + return 0;
> +
> + pcie = dev_get_drvdata(dev);
> /*
> * Set minimum bandwidth required to keep data path functional during
> * suspend.
> @@ -1795,9 +1888,13 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>
> static int qcom_pcie_resume_noirq(struct device *dev)
> {
> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
> + struct qcom_pcie *pcie;
> int ret;
>
> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
Ditto
> + return 0;
> +
> + pcie = dev_get_drvdata(dev);
> if (pm_suspend_target_state != PM_SUSPEND_MEM) {
> ret = icc_enable(pcie->icc_cpu);
> if (ret) {
> @@ -1830,6 +1927,7 @@ static const struct of_device_id qcom_pcie_match[] = {
> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> + { .compatible = "qcom,pcie-sa8255p", .data = &cfg_fw_managed },
> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
> { .compatible = "qcom,pcie-sa8775p", .data = &cfg_1_34_0},
> { .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
Thanks,
Neil
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex
2024-11-06 22:13 ` [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex Mayank Rana
@ 2024-11-07 9:35 ` Krzysztof Kozlowski
2024-11-07 17:39 ` Mayank Rana
2024-11-15 10:55 ` Manivannan Sadhasivam
1 sibling, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-07 9:35 UTC (permalink / raw)
To: Mayank Rana, jingoohan1, manivannan.sadhasivam, will, lpieralisi,
kw, robh, bhelgaas
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai
On 06/11/2024 23:13, Mayank Rana wrote:
> On SA8255p, PCIe root complex is managed by firmware using power-domain
> based handling. This root complex is configured as ECAM compliant.
> Document required configuration to enable PCIe root complex.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> .../bindings/pci/qcom,pcie-sa8255p.yaml | 103 ++++++++++++++++++
NAK
<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.
You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.
Please kindly resend and include all necessary To/Cc entries.
</form letter>
BTW, you also Cc-ed incorrect addresses :/
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex
2024-11-07 9:35 ` Krzysztof Kozlowski
@ 2024-11-07 17:39 ` Mayank Rana
0 siblings, 0 replies; 24+ messages in thread
From: Mayank Rana @ 2024-11-07 17:39 UTC (permalink / raw)
To: Krzysztof Kozlowski, jingoohan1, manivannan.sadhasivam, will,
lpieralisi, kw, robh, bhelgaas
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai
On 11/7/2024 1:35 AM, Krzysztof Kozlowski wrote:
> On 06/11/2024 23:13, Mayank Rana wrote:
>> On SA8255p, PCIe root complex is managed by firmware using power-domain
>> based handling. This root complex is configured as ECAM compliant.
>> Document required configuration to enable PCIe root complex.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> .../bindings/pci/qcom,pcie-sa8255p.yaml | 103 ++++++++++++++++++
>
> NAK
>
> <form letter>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline) or work on fork of kernel
> (don't, instead use mainline). Just use b4 and everything should be
> fine, although remember about `b4 prep --auto-to-cc` if you added new
> patches to the patchset.
>
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.
>
> Please kindly resend and include all necessary To/Cc entries.
> </form letter>
>
> BTW, you also Cc-ed incorrect addresses :/
sorry, I missed adding right set of folks and mailing list here.
I shall resend after updating correct reviewers.
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-07 8:45 ` neil.armstrong
@ 2024-11-07 17:45 ` Mayank Rana
2024-11-08 10:22 ` neil.armstrong
0 siblings, 1 reply; 24+ messages in thread
From: Mayank Rana @ 2024-11-07 17:45 UTC (permalink / raw)
To: neil.armstrong, jingoohan1, manivannan.sadhasivam, will,
lpieralisi, kw, robh, bhelgaas, krzk
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai
On 11/7/2024 12:45 AM, neil.armstrong@linaro.org wrote:
> Hi,
>
> On 06/11/2024 23:13, Mayank Rana wrote:
>> On SA8255p ride platform, PCIe root complex is firmware managed as well
>> configured into ECAM compliant mode. This change adds functionality to
>> enable resource management (system resource as well PCIe controller and
>> PHY configuration) through firmware, and enumerating ECAM compliant root
>> complex.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> drivers/pci/controller/dwc/Kconfig | 1 +
>> drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
>> 2 files changed, 108 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig
>> b/drivers/pci/controller/dwc/Kconfig
>> index b6d6778b0698..0fe76bd39d69 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -275,6 +275,7 @@ config PCIE_QCOM
>> select PCIE_DW_HOST
>> select CRC8
>> select PCIE_QCOM_COMMON
>> + select PCI_HOST_COMMON
>> help
>> Say Y here to enable PCIe controller support on Qualcomm SoCs.
>> The
>> PCIe controller uses the DesignWare core plus Qualcomm-specific
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
>> b/drivers/pci/controller/dwc/pcie-qcom.c
>> index ef44a82be058..2cb74f902baf 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -21,7 +21,9 @@
>> #include <linux/limits.h>
>> #include <linux/init.h>
>> #include <linux/of.h>
>> +#include <linux/of_pci.h>
>> #include <linux/pci.h>
>> +#include <linux/pci-ecam.h>
>> #include <linux/pm_opp.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/platform_device.h>
>> @@ -254,10 +256,12 @@ struct qcom_pcie_ops {
>> * @ops: qcom PCIe ops structure
>> * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable
>> cache
>> * snooping
>> + * @firmware_managed: Set if PCIe root complex is firmware managed
>> */
>> struct qcom_pcie_cfg {
>> const struct qcom_pcie_ops *ops;
>> bool override_no_snoop;
>> + bool firmware_managed;
>> bool no_l0s;
>> };
>> @@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>> .no_l0s = true,
>> };
>> +static const struct qcom_pcie_cfg cfg_fw_managed = {
>> + .firmware_managed = true,
>> +};
>> +
>> static const struct dw_pcie_ops dw_pcie_ops = {
>> .link_up = qcom_pcie_link_up,
>> .start_link = qcom_pcie_start_link,
>> @@ -1566,6 +1574,51 @@ static irqreturn_t
>> qcom_pcie_global_irq_thread(int irq, void *data)
>> return IRQ_HANDLED;
>> }
>> +static void qcom_pci_free_msi(void *ptr)
>> +{
>> + struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
>> +
>> + if (pp && pp->has_msi_ctrl)
>> + dw_pcie_free_msi(pp);
>> +}
>> +
>> +static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
>> +{
>> + struct device *dev = cfg->parent;
>> + struct dw_pcie_rp *pp;
>> + struct dw_pcie *pci;
>> + int ret;
>> +
>> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>> + if (!pci)
>> + return -ENOMEM;
>> +
>> + pci->dev = dev;
>> + pp = &pci->pp;
>> + pci->dbi_base = cfg->win;
>> + pp->num_vectors = MSI_DEF_NUM_VECTORS;
>> +
>> + ret = dw_pcie_msi_host_init(pp);
>> + if (ret)
>> + return ret;
>> +
>> + pp->has_msi_ctrl = true;
>> + dw_pcie_msi_init(pp);
>> +
>> + ret = devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
>> + return ret;
>> +}
>> +
>> +/* ECAM ops */
>> +const struct pci_ecam_ops pci_qcom_ecam_ops = {
>> + .init = qcom_pcie_ecam_host_init,
>> + .pci_ops = {
>> + .map_bus = pci_ecam_map_bus,
>> + .read = pci_generic_config_read,
>> + .write = pci_generic_config_write,
>> + }
>> +};
>> +
>> static int qcom_pcie_probe(struct platform_device *pdev)
>> {
>> const struct qcom_pcie_cfg *pcie_cfg;
>> @@ -1580,11 +1633,52 @@ static int qcom_pcie_probe(struct
>> platform_device *pdev)
>> char *name;
>> pcie_cfg = of_device_get_match_data(dev);
>> - if (!pcie_cfg || !pcie_cfg->ops) {
>> - dev_err(dev, "Invalid platform data\n");
>> + if (!pcie_cfg) {
>> + dev_err(dev, "No platform data\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (!pcie_cfg->firmware_managed && !pcie_cfg->ops) {
>> + dev_err(dev, "No platform ops\n");
>> return -EINVAL;
>> }
>> + pm_runtime_enable(dev);
>> + ret = pm_runtime_get_sync(dev);
>> + if (ret < 0)
>> + goto err_pm_runtime_put;
>> +
>> + if (pcie_cfg->firmware_managed) {
>> + struct pci_host_bridge *bridge;
>> + struct pci_config_window *cfg;
>> +
>> + bridge = devm_pci_alloc_host_bridge(dev, 0);
>> + if (!bridge) {
>> + ret = -ENOMEM;
>> + goto err_pm_runtime_put;
>> + }
>> +
>> + of_pci_check_probe_only();
>> + /* Parse and map our Configuration Space windows */
>> + cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
>> + if (IS_ERR(cfg)) {
>> + ret = PTR_ERR(cfg);
>> + goto err_pm_runtime_put;
>> + }
>> +
>> + bridge->sysdata = cfg;
>> + bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
>> + bridge->msi_domain = true;
>> +
>> + ret = pci_host_probe(bridge);
>> + if (ret) {
>> + dev_err(dev, "pci_host_probe() failed:%d\n", ret);
>> + goto err_pm_runtime_put;
>> + }
>> +
>> + return ret;
>> + }
>> +
>> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>> if (!pcie)
>> return -ENOMEM;
>> @@ -1593,11 +1687,6 @@ static int qcom_pcie_probe(struct
>> platform_device *pdev)
>> if (!pci)
>> return -ENOMEM;
>> - pm_runtime_enable(dev);
>> - ret = pm_runtime_get_sync(dev);
>> - if (ret < 0)
>> - goto err_pm_runtime_put;
>> -
>> pci->dev = dev;
>> pci->ops = &dw_pcie_ops;
>> pp = &pci->pp;
>> @@ -1739,9 +1828,13 @@ static int qcom_pcie_probe(struct
>> platform_device *pdev)
>> static int qcom_pcie_suspend_noirq(struct device *dev)
>> {
>> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
>> + struct qcom_pcie *pcie;
>> int ret = 0;
>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
>
> Can't you use if (pcie_cfg->firmware_managed) here instead ?
yes, although with firmware managed mode, struct qcom_pcie *pcie is not
allocated, and just
to get access to pcie_cfg for this check, I took this approach. I am
thiking to do allocating struct qcom_pcie *pcie and using it in future
if we need more other related functionality which needs usage of this
structure for functionality like global interrupt etc.
Although if you still prefer to allocate struct qcom_pcie based memory
to access pcie_cfg, then I can consider to update in next patchset.
Please suggest.
>> + return 0;
>> +
>> + pcie = dev_get_drvdata(dev);
>> /*
>> * Set minimum bandwidth required to keep data path functional
>> during
>> * suspend.
>> @@ -1795,9 +1888,13 @@ static int qcom_pcie_suspend_noirq(struct
>> device *dev)
>> static int qcom_pcie_resume_noirq(struct device *dev)
>> {
>> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
>> + struct qcom_pcie *pcie;
>> int ret;
>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
>
> Ditto
>
>> + return 0;
>> +
>> + pcie = dev_get_drvdata(dev);
>> if (pm_suspend_target_state != PM_SUSPEND_MEM) {
>> ret = icc_enable(pcie->icc_cpu);
>> if (ret) {
>> @@ -1830,6 +1927,7 @@ static const struct of_device_id
>> qcom_pcie_match[] = {
>> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
>> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
>> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
>> + { .compatible = "qcom,pcie-sa8255p", .data = &cfg_fw_managed },
>> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
>> { .compatible = "qcom,pcie-sa8775p", .data = &cfg_1_34_0},
>> { .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
>
> Thanks,
> Neil
Regards,
Mayank
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-07 17:45 ` Mayank Rana
@ 2024-11-08 10:22 ` neil.armstrong
2024-11-15 11:21 ` Manivannan Sadhasivam
0 siblings, 1 reply; 24+ messages in thread
From: neil.armstrong @ 2024-11-08 10:22 UTC (permalink / raw)
To: Mayank Rana, jingoohan1, manivannan.sadhasivam, will, lpieralisi,
kw, robh, bhelgaas, krzk
Cc: linux-pci, linux-arm-msm, linux-kernel, quic_krichai
On 07/11/2024 18:45, Mayank Rana wrote:
>
>
> On 11/7/2024 12:45 AM, neil.armstrong@linaro.org wrote:
>> Hi,
>>
>> On 06/11/2024 23:13, Mayank Rana wrote:
>>> On SA8255p ride platform, PCIe root complex is firmware managed as well
>>> configured into ECAM compliant mode. This change adds functionality to
>>> enable resource management (system resource as well PCIe controller and
>>> PHY configuration) through firmware, and enumerating ECAM compliant root
>>> complex.
>>>
>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>> ---
>>> drivers/pci/controller/dwc/Kconfig | 1 +
>>> drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
>>> 2 files changed, 108 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>>> index b6d6778b0698..0fe76bd39d69 100644
>>> --- a/drivers/pci/controller/dwc/Kconfig
>>> +++ b/drivers/pci/controller/dwc/Kconfig
>>> @@ -275,6 +275,7 @@ config PCIE_QCOM
>>> select PCIE_DW_HOST
>>> select CRC8
>>> select PCIE_QCOM_COMMON
>>> + select PCI_HOST_COMMON
>>> help
>>> Say Y here to enable PCIe controller support on Qualcomm SoCs. The
>>> PCIe controller uses the DesignWare core plus Qualcomm-specific
>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>> index ef44a82be058..2cb74f902baf 100644
>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>> @@ -21,7 +21,9 @@
>>> #include <linux/limits.h>
>>> #include <linux/init.h>
>>> #include <linux/of.h>
>>> +#include <linux/of_pci.h>
>>> #include <linux/pci.h>
>>> +#include <linux/pci-ecam.h>
>>> #include <linux/pm_opp.h>
>>> #include <linux/pm_runtime.h>
>>> #include <linux/platform_device.h>
>>> @@ -254,10 +256,12 @@ struct qcom_pcie_ops {
>>> * @ops: qcom PCIe ops structure
>>> * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
>>> * snooping
>>> + * @firmware_managed: Set if PCIe root complex is firmware managed
>>> */
>>> struct qcom_pcie_cfg {
>>> const struct qcom_pcie_ops *ops;
>>> bool override_no_snoop;
>>> + bool firmware_managed;
>>> bool no_l0s;
>>> };
>>> @@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>>> .no_l0s = true,
>>> };
>>> +static const struct qcom_pcie_cfg cfg_fw_managed = {
>>> + .firmware_managed = true,
>>> +};
>>> +
>>> static const struct dw_pcie_ops dw_pcie_ops = {
>>> .link_up = qcom_pcie_link_up,
>>> .start_link = qcom_pcie_start_link,
>>> @@ -1566,6 +1574,51 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
>>> return IRQ_HANDLED;
>>> }
>>> +static void qcom_pci_free_msi(void *ptr)
>>> +{
>>> + struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
>>> +
>>> + if (pp && pp->has_msi_ctrl)
>>> + dw_pcie_free_msi(pp);
>>> +}
>>> +
>>> +static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
>>> +{
>>> + struct device *dev = cfg->parent;
>>> + struct dw_pcie_rp *pp;
>>> + struct dw_pcie *pci;
>>> + int ret;
>>> +
>>> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>>> + if (!pci)
>>> + return -ENOMEM;
>>> +
>>> + pci->dev = dev;
>>> + pp = &pci->pp;
>>> + pci->dbi_base = cfg->win;
>>> + pp->num_vectors = MSI_DEF_NUM_VECTORS;
>>> +
>>> + ret = dw_pcie_msi_host_init(pp);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + pp->has_msi_ctrl = true;
>>> + dw_pcie_msi_init(pp);
>>> +
>>> + ret = devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
>>> + return ret;
>>> +}
>>> +
>>> +/* ECAM ops */
>>> +const struct pci_ecam_ops pci_qcom_ecam_ops = {
>>> + .init = qcom_pcie_ecam_host_init,
>>> + .pci_ops = {
>>> + .map_bus = pci_ecam_map_bus,
>>> + .read = pci_generic_config_read,
>>> + .write = pci_generic_config_write,
>>> + }
>>> +};
>>> +
>>> static int qcom_pcie_probe(struct platform_device *pdev)
>>> {
>>> const struct qcom_pcie_cfg *pcie_cfg;
>>> @@ -1580,11 +1633,52 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>> char *name;
>>> pcie_cfg = of_device_get_match_data(dev);
>>> - if (!pcie_cfg || !pcie_cfg->ops) {
>>> - dev_err(dev, "Invalid platform data\n");
>>> + if (!pcie_cfg) {
>>> + dev_err(dev, "No platform data\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + if (!pcie_cfg->firmware_managed && !pcie_cfg->ops) {
>>> + dev_err(dev, "No platform ops\n");
>>> return -EINVAL;
>>> }
>>> + pm_runtime_enable(dev);
>>> + ret = pm_runtime_get_sync(dev);
>>> + if (ret < 0)
>>> + goto err_pm_runtime_put;
>>> +
>>> + if (pcie_cfg->firmware_managed) {
>>> + struct pci_host_bridge *bridge;
>>> + struct pci_config_window *cfg;
>>> +
>>> + bridge = devm_pci_alloc_host_bridge(dev, 0);
>>> + if (!bridge) {
>>> + ret = -ENOMEM;
>>> + goto err_pm_runtime_put;
>>> + }
>>> +
>>> + of_pci_check_probe_only();
>>> + /* Parse and map our Configuration Space windows */
>>> + cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
>>> + if (IS_ERR(cfg)) {
>>> + ret = PTR_ERR(cfg);
>>> + goto err_pm_runtime_put;
>>> + }
>>> +
>>> + bridge->sysdata = cfg;
>>> + bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
>>> + bridge->msi_domain = true;
>>> +
>>> + ret = pci_host_probe(bridge);
>>> + if (ret) {
>>> + dev_err(dev, "pci_host_probe() failed:%d\n", ret);
>>> + goto err_pm_runtime_put;
>>> + }
>>> +
>>> + return ret;
>>> + }
>>> +
>>> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>>> if (!pcie)
>>> return -ENOMEM;
>>> @@ -1593,11 +1687,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>> if (!pci)
>>> return -ENOMEM;
>>> - pm_runtime_enable(dev);
>>> - ret = pm_runtime_get_sync(dev);
>>> - if (ret < 0)
>>> - goto err_pm_runtime_put;
>>> -
>>> pci->dev = dev;
>>> pci->ops = &dw_pcie_ops;
>>> pp = &pci->pp;
>>> @@ -1739,9 +1828,13 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>> static int qcom_pcie_suspend_noirq(struct device *dev)
>>> {
>>> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
>>> + struct qcom_pcie *pcie;
>>> int ret = 0;
>>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
>>
>> Can't you use if (pcie_cfg->firmware_managed) here instead ?
> yes, although with firmware managed mode, struct qcom_pcie *pcie is not allocated, and just
> to get access to pcie_cfg for this check, I took this approach. I am thiking to do allocating struct qcom_pcie *pcie and using it in future if we need more other related functionality which needs usage of this structure for functionality like global interrupt etc.
>
> Although if you still prefer to allocate struct qcom_pcie based memory to access pcie_cfg, then I can consider to update in next patchset. Please suggest.
I understand, but running of_device_is_compatible() in runtime PM is not something we should do,
so either allocate pcie_cfg, or add a firmware_managed bool to qcom_pcie copied from pcie_cfg,
or move runtime pm callbacks in qcom_pcie_ops and don't declare any in cfg_fw_managed->ops.
I think the latter would be more scalable so we could add runtime pm variant handling
for each IP versions. But it may be quite quite useless for now.
I'll leave Mani comment on that.
Neil
>>> + return 0;
>>> +
>>> + pcie = dev_get_drvdata(dev);
>>> /*
>>> * Set minimum bandwidth required to keep data path functional during
>>> * suspend.
>>> @@ -1795,9 +1888,13 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>>> static int qcom_pcie_resume_noirq(struct device *dev)
>>> {
>>> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
>>> + struct qcom_pcie *pcie;
>>> int ret;
>>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
>>
>> Ditto
>>
>>> + return 0;
>>> +
>>> + pcie = dev_get_drvdata(dev);
>>> if (pm_suspend_target_state != PM_SUSPEND_MEM) {
>>> ret = icc_enable(pcie->icc_cpu);
>>> if (ret) {
>>> @@ -1830,6 +1927,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>>> { .compatible = "qcom,pcie-ipq8074-gen3", .data = &cfg_2_9_0 },
>>> { .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
>>> { .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
>>> + { .compatible = "qcom,pcie-sa8255p", .data = &cfg_fw_managed },
>>> { .compatible = "qcom,pcie-sa8540p", .data = &cfg_sc8280xp },
>>> { .compatible = "qcom,pcie-sa8775p", .data = &cfg_1_34_0},
>>> { .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
>>
>> Thanks,
>> Neil
>
> Regards,
> Mayank
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-06 22:13 ` [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality Mayank Rana
2024-11-07 8:45 ` neil.armstrong
@ 2024-11-09 23:45 ` kernel test robot
2024-11-15 11:25 ` Manivannan Sadhasivam
2 siblings, 0 replies; 24+ messages in thread
From: kernel test robot @ 2024-11-09 23:45 UTC (permalink / raw)
To: Mayank Rana, jingoohan1, manivannan.sadhasivam, will, lpieralisi,
kw, robh, bhelgaas, krzk
Cc: oe-kbuild-all, linux-pci, linux-arm-msm, linux-kernel,
quic_krichai, Mayank Rana
Hi Mayank,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus mani-mhi/mhi-next robh/for-next linus/master v6.12-rc6 next-20241108]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mayank-Rana/PCI-dwc-Export-dwc-MSI-controller-related-APIs/20241107-061634
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20241106221341.2218416-5-quic_mrana%40quicinc.com
patch subject: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
config: powerpc-randconfig-r131-20241109 (https://download.01.org/0day-ci/archive/20241110/202411100738.kCgjRkIv-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce: (https://download.01.org/0day-ci/archive/20241110/202411100738.kCgjRkIv-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411100738.kCgjRkIv-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/pci/controller/dwc/pcie-qcom.c:1613:27: sparse: sparse: symbol 'pci_qcom_ecam_ops' was not declared. Should it be static?
vim +/pci_qcom_ecam_ops +1613 drivers/pci/controller/dwc/pcie-qcom.c
1611
1612 /* ECAM ops */
> 1613 const struct pci_ecam_ops pci_qcom_ecam_ops = {
1614 .init = qcom_pcie_ecam_host_init,
1615 .pci_ops = {
1616 .map_bus = pci_ecam_map_bus,
1617 .read = pci_generic_config_read,
1618 .write = pci_generic_config_write,
1619 }
1620 };
1621
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/4] PCI: dwc: Export dwc MSI controller related APIs
2024-11-06 22:13 ` [PATCH v3 1/4] PCI: dwc: Export dwc MSI controller related APIs Mayank Rana
@ 2024-11-15 9:14 ` Manivannan Sadhasivam
2024-11-15 18:15 ` Mayank Rana
0 siblings, 1 reply; 24+ messages in thread
From: Manivannan Sadhasivam @ 2024-11-15 9:14 UTC (permalink / raw)
To: Mayank Rana
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On Wed, Nov 06, 2024 at 02:13:38PM -0800, Mayank Rana wrote:
> To allow dwc PCIe controller based MSI functionality from ECAM pcie
> driver, export dw_pcie_msi_host_init(), dw_pcie_msi_init() and
> dw_pcie_msi_free() APIs. Also move MSI IRQ related initialization code
> into dw_pcie_msi_init() as this code executes before dw_pcie_msi_init()
> API to use with ECAM driver.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> .../pci/controller/dwc/pcie-designware-host.c | 38 ++++++++++---------
> drivers/pci/controller/dwc/pcie-designware.h | 14 +++++++
> 2 files changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 3e41865c7290..25020a090db8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -250,7 +250,7 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
> return 0;
> }
>
> -static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
> +void dw_pcie_free_msi(struct dw_pcie_rp *pp)
> {
> u32 ctrl;
>
> @@ -263,19 +263,34 @@ static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
> irq_domain_remove(pp->msi_domain);
> irq_domain_remove(pp->irq_domain);
> }
> +EXPORT_SYMBOL_GPL(dw_pcie_free_msi);
>
> -static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
> +void dw_pcie_msi_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> u64 msi_target = (u64)pp->msi_data;
> + u32 ctrl, num_ctrls;
>
> if (!pci_msi_enabled() || !pp->has_msi_ctrl)
> return;
>
> + num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +
> + /* Initialize IRQ Status array */
> + for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
> + (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> + pp->irq_mask[ctrl]);
> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
> + (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> + ~0);
> + }
> +
> /* Program the msi_data */
> dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
> dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
> }
> +EXPORT_SYMBOL_GPL(dw_pcie_msi_init);
>
> static int dw_pcie_parse_split_msi_irq(struct dw_pcie_rp *pp)
> {
> @@ -317,7 +332,7 @@ static int dw_pcie_parse_split_msi_irq(struct dw_pcie_rp *pp)
> return 0;
> }
>
> -static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> +int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct device *dev = pci->dev;
> @@ -391,6 +406,7 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(dw_pcie_msi_host_init);
>
> static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> {
> @@ -802,7 +818,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - u32 val, ctrl, num_ctrls;
> + u32 val;
> int ret;
>
> /*
> @@ -813,20 +829,6 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
>
> dw_pcie_setup(pci);
>
> - if (pp->has_msi_ctrl) {
> - num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> -
> - /* Initialize IRQ Status array */
> - for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
> - (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> - pp->irq_mask[ctrl]);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
> - (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> - ~0);
> - }
> - }
> -
> dw_pcie_msi_init(pp);
>
> /* Setup RC BARs */
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 347ab74ac35a..ef748d82c663 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -679,6 +679,9 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
>
> #ifdef CONFIG_PCIE_DW_HOST
> irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
> +void dw_pcie_msi_init(struct dw_pcie_rp *pp);
> +int dw_pcie_msi_host_init(struct dw_pcie_rp *pp);
> +void dw_pcie_free_msi(struct dw_pcie_rp *pp);
> int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> int dw_pcie_host_init(struct dw_pcie_rp *pp);
> void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
> @@ -691,6 +694,17 @@ static inline irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp)
> return IRQ_NONE;
> }
>
> +static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
Missing 'inline' here and below?
- Mani
> +{ }
> +
> +static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
> +{
> + return -ENODEV;
> +}
> +
> +static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
> +{ }
> +
> static inline int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> {
> return 0;
> --
> 2.25.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/4] PCI: host-generic: Export gen_pci_init() API to allow ECAM creation
2024-11-06 22:13 ` [PATCH v3 2/4] PCI: host-generic: Export gen_pci_init() API to allow ECAM creation Mayank Rana
@ 2024-11-15 9:17 ` Manivannan Sadhasivam
2024-11-15 18:16 ` Mayank Rana
0 siblings, 1 reply; 24+ messages in thread
From: Manivannan Sadhasivam @ 2024-11-15 9:17 UTC (permalink / raw)
To: Mayank Rana
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On Wed, Nov 06, 2024 at 02:13:39PM -0800, Mayank Rana wrote:
> Export gen_pci_init() API to create ECAM and initialized ECAM OPs
> from PCIe driver which don't have way to populate driver_data as
> just ECAM ops.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/pci-host-common.c | 3 ++-
> include/linux/pci-ecam.h | 2 ++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index cf5f59a745b3..b9460a4c5b7e 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -20,7 +20,7 @@ static void gen_pci_unmap_cfg(void *ptr)
> pci_ecam_free((struct pci_config_window *)ptr);
> }
>
> -static struct pci_config_window *gen_pci_init(struct device *dev,
> +struct pci_config_window *gen_pci_init(struct device *dev,
Please rename the API to something like 'pci_host_common_init()'.
'gen_pci_init()' is fine within the driver, but doesn't look good when exported
outside.
- Mani
> struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops)
> {
> int err;
> @@ -48,6 +48,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
>
> return cfg;
> }
> +EXPORT_SYMBOL_GPL(gen_pci_init);
>
> int pci_host_common_probe(struct platform_device *pdev)
> {
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 3a4860bd2758..386c08349169 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -94,5 +94,7 @@ extern const struct pci_ecam_ops loongson_pci_ecam_ops; /* Loongson PCIe */
> /* for DT-based PCI controllers that support ECAM */
> int pci_host_common_probe(struct platform_device *pdev);
> void pci_host_common_remove(struct platform_device *pdev);
> +struct pci_config_window *gen_pci_init(struct device *dev,
> + struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
> #endif
> #endif
> --
> 2.25.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex
2024-11-06 22:13 ` [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex Mayank Rana
2024-11-07 9:35 ` Krzysztof Kozlowski
@ 2024-11-15 10:55 ` Manivannan Sadhasivam
1 sibling, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2024-11-15 10:55 UTC (permalink / raw)
To: Mayank Rana
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On Wed, Nov 06, 2024 at 02:13:40PM -0800, Mayank Rana wrote:
> On SA8255p, PCIe root complex is managed by firmware using power-domain
> based handling. This root complex is configured as ECAM compliant.
> Document required configuration to enable PCIe root complex.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> .../bindings/pci/qcom,pcie-sa8255p.yaml | 103 ++++++++++++++++++
> 1 file changed, 103 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
> new file mode 100644
> index 000000000000..9b09c3923ba0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/qcom,pcie-sa8255p.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SA8255p based firmware managed and ECAM compliant PCIe Root Complex
> +
> +maintainers:
> + - Bjorn Andersson <andersson@kernel.org>
> + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> +
> +description:
> + Qualcomm SA8255p SoC PCIe root complex controller is based on the Synopsys
> + DesignWare PCIe IP which is managed by firmware, and configured in ECAM mode.
> +
> +properties:
> + compatible:
> + const: qcom,pcie-sa8255p
> +
> + reg:
> + description:
> + The Configuration Space base address and size, as accessed from the parent
> + bus. The base address corresponds to the first bus in the "bus-range"
> + property. If no "bus-range" is specified, this will be bus 0 (the
> + default).
I don't think the 'no bus-range' configuration is supported. You can get rid if
this statement.
> + maxItems: 1
> +
> + ranges:
> + description:
> + As described in IEEE Std 1275-1994, but must provide at least a
> + definition of non-prefetchable memory. One or both of prefetchable Memory
> + may also be provided.
> + minItems: 1
> + maxItems: 2
> +
> + interrupts:
> + minItems: 8
> + maxItems: 8
> +
> + interrupt-names:
> + items:
> + - const: msi0
> + - const: msi1
> + - const: msi2
> + - const: msi3
> + - const: msi4
> + - const: msi5
> + - const: msi6
> + - const: msi7
> +
> + power-domains:
> + maxItems: 1
> +
> + dma-coherent: true
> + iommu-map: true
Can't you add 'msi-map' also since the hardware supports it?
- Mani
> +
> +required:
> + - compatible
> + - reg
> + - ranges
> + - power-domains
> +
> +allOf:
> + - $ref: /schemas/pci/pci-host-bridge.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pci@1c00000 {
> + compatible = "qcom,pcie-sa8255p";
> + reg = <0x4 0x00000000 0 0x10000000>;
> + device_type = "pci";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x02000000 0x0 0x40100000 0x0 0x40100000 0x0 0x1ff00000>,
> + <0x43000000 0x4 0x10100000 0x4 0x10100000 0x0 0x40000000>;
> + bus-range = <0x00 0xff>;
> + dma-coherent;
> + linux,pci-domain = <0>;
> + power-domains = <&scmi5_pd 0>;
> + iommu-map = <0x0 &pcie_smmu 0x0000 0x1>,
> + <0x100 &pcie_smmu 0x0001 0x1>;
> + interrupt-parent = <&intc>;
> + interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi0", "msi1", "msi2", "msi3",
> + "msi4", "msi5", "msi6", "msi7";
> + };
> + };
> --
> 2.25.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-08 10:22 ` neil.armstrong
@ 2024-11-15 11:21 ` Manivannan Sadhasivam
2024-11-15 18:17 ` Mayank Rana
0 siblings, 1 reply; 24+ messages in thread
From: Manivannan Sadhasivam @ 2024-11-15 11:21 UTC (permalink / raw)
To: neil.armstrong
Cc: Mayank Rana, jingoohan1, will, lpieralisi, kw, robh, bhelgaas,
krzk, linux-pci, linux-arm-msm, linux-kernel, quic_krichai
On Fri, Nov 08, 2024 at 11:22:52AM +0100, neil.armstrong@linaro.org wrote:
> On 07/11/2024 18:45, Mayank Rana wrote:
> >
> >
> > On 11/7/2024 12:45 AM, neil.armstrong@linaro.org wrote:
> > > Hi,
> > >
> > > On 06/11/2024 23:13, Mayank Rana wrote:
> > > > On SA8255p ride platform, PCIe root complex is firmware managed as well
> > > > configured into ECAM compliant mode. This change adds functionality to
> > > > enable resource management (system resource as well PCIe controller and
> > > > PHY configuration) through firmware, and enumerating ECAM compliant root
> > > > complex.
> > > >
> > > > Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> > > > ---
> > > > drivers/pci/controller/dwc/Kconfig | 1 +
> > > > drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
> > > > 2 files changed, 108 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > > index b6d6778b0698..0fe76bd39d69 100644
> > > > --- a/drivers/pci/controller/dwc/Kconfig
> > > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > > @@ -275,6 +275,7 @@ config PCIE_QCOM
> > > > select PCIE_DW_HOST
> > > > select CRC8
> > > > select PCIE_QCOM_COMMON
> > > > + select PCI_HOST_COMMON
> > > > help
> > > > Say Y here to enable PCIe controller support on Qualcomm SoCs. The
> > > > PCIe controller uses the DesignWare core plus Qualcomm-specific
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > index ef44a82be058..2cb74f902baf 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > @@ -21,7 +21,9 @@
> > > > #include <linux/limits.h>
> > > > #include <linux/init.h>
> > > > #include <linux/of.h>
> > > > +#include <linux/of_pci.h>
> > > > #include <linux/pci.h>
> > > > +#include <linux/pci-ecam.h>
> > > > #include <linux/pm_opp.h>
> > > > #include <linux/pm_runtime.h>
> > > > #include <linux/platform_device.h>
> > > > @@ -254,10 +256,12 @@ struct qcom_pcie_ops {
> > > > * @ops: qcom PCIe ops structure
> > > > * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
> > > > * snooping
> > > > + * @firmware_managed: Set if PCIe root complex is firmware managed
> > > > */
> > > > struct qcom_pcie_cfg {
> > > > const struct qcom_pcie_ops *ops;
> > > > bool override_no_snoop;
> > > > + bool firmware_managed;
> > > > bool no_l0s;
> > > > };
> > > > @@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
> > > > .no_l0s = true,
> > > > };
> > > > +static const struct qcom_pcie_cfg cfg_fw_managed = {
> > > > + .firmware_managed = true,
> > > > +};
> > > > +
> > > > static const struct dw_pcie_ops dw_pcie_ops = {
> > > > .link_up = qcom_pcie_link_up,
> > > > .start_link = qcom_pcie_start_link,
> > > > @@ -1566,6 +1574,51 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
> > > > return IRQ_HANDLED;
> > > > }
> > > > +static void qcom_pci_free_msi(void *ptr)
> > > > +{
> > > > + struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
> > > > +
> > > > + if (pp && pp->has_msi_ctrl)
> > > > + dw_pcie_free_msi(pp);
> > > > +}
> > > > +
> > > > +static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
> > > > +{
> > > > + struct device *dev = cfg->parent;
> > > > + struct dw_pcie_rp *pp;
> > > > + struct dw_pcie *pci;
> > > > + int ret;
> > > > +
> > > > + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> > > > + if (!pci)
> > > > + return -ENOMEM;
> > > > +
> > > > + pci->dev = dev;
> > > > + pp = &pci->pp;
> > > > + pci->dbi_base = cfg->win;
> > > > + pp->num_vectors = MSI_DEF_NUM_VECTORS;
> > > > +
> > > > + ret = dw_pcie_msi_host_init(pp);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + pp->has_msi_ctrl = true;
> > > > + dw_pcie_msi_init(pp);
> > > > +
> > > > + ret = devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +/* ECAM ops */
> > > > +const struct pci_ecam_ops pci_qcom_ecam_ops = {
> > > > + .init = qcom_pcie_ecam_host_init,
> > > > + .pci_ops = {
> > > > + .map_bus = pci_ecam_map_bus,
> > > > + .read = pci_generic_config_read,
> > > > + .write = pci_generic_config_write,
> > > > + }
> > > > +};
> > > > +
> > > > static int qcom_pcie_probe(struct platform_device *pdev)
> > > > {
> > > > const struct qcom_pcie_cfg *pcie_cfg;
> > > > @@ -1580,11 +1633,52 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > > > char *name;
> > > > pcie_cfg = of_device_get_match_data(dev);
> > > > - if (!pcie_cfg || !pcie_cfg->ops) {
> > > > - dev_err(dev, "Invalid platform data\n");
> > > > + if (!pcie_cfg) {
> > > > + dev_err(dev, "No platform data\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (!pcie_cfg->firmware_managed && !pcie_cfg->ops) {
> > > > + dev_err(dev, "No platform ops\n");
> > > > return -EINVAL;
> > > > }
> > > > + pm_runtime_enable(dev);
> > > > + ret = pm_runtime_get_sync(dev);
> > > > + if (ret < 0)
> > > > + goto err_pm_runtime_put;
> > > > +
> > > > + if (pcie_cfg->firmware_managed) {
> > > > + struct pci_host_bridge *bridge;
> > > > + struct pci_config_window *cfg;
> > > > +
> > > > + bridge = devm_pci_alloc_host_bridge(dev, 0);
> > > > + if (!bridge) {
> > > > + ret = -ENOMEM;
> > > > + goto err_pm_runtime_put;
> > > > + }
> > > > +
> > > > + of_pci_check_probe_only();
> > > > + /* Parse and map our Configuration Space windows */
> > > > + cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
> > > > + if (IS_ERR(cfg)) {
> > > > + ret = PTR_ERR(cfg);
> > > > + goto err_pm_runtime_put;
> > > > + }
> > > > +
> > > > + bridge->sysdata = cfg;
> > > > + bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
> > > > + bridge->msi_domain = true;
> > > > +
> > > > + ret = pci_host_probe(bridge);
> > > > + if (ret) {
> > > > + dev_err(dev, "pci_host_probe() failed:%d\n", ret);
> > > > + goto err_pm_runtime_put;
> > > > + }
> > > > +
> > > > + return ret;
> > > > + }
> > > > +
> > > > pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> > > > if (!pcie)
> > > > return -ENOMEM;
> > > > @@ -1593,11 +1687,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > > > if (!pci)
> > > > return -ENOMEM;
> > > > - pm_runtime_enable(dev);
> > > > - ret = pm_runtime_get_sync(dev);
> > > > - if (ret < 0)
> > > > - goto err_pm_runtime_put;
> > > > -
> > > > pci->dev = dev;
> > > > pci->ops = &dw_pcie_ops;
> > > > pp = &pci->pp;
> > > > @@ -1739,9 +1828,13 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > > > static int qcom_pcie_suspend_noirq(struct device *dev)
> > > > {
> > > > - struct qcom_pcie *pcie = dev_get_drvdata(dev);
> > > > + struct qcom_pcie *pcie;
> > > > int ret = 0;
> > > > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
> > >
> > > Can't you use if (pcie_cfg->firmware_managed) here instead ?
> > yes, although with firmware managed mode, struct qcom_pcie *pcie is not allocated, and just
> > to get access to pcie_cfg for this check, I took this approach. I am thiking to do allocating struct qcom_pcie *pcie and using it in future if we need more other related functionality which needs usage of this structure for functionality like global interrupt etc.
> >
> > Although if you still prefer to allocate struct qcom_pcie based memory to access pcie_cfg, then I can consider to update in next patchset. Please suggest.
>
> I understand, but running of_device_is_compatible() in runtime PM is not something we should do,
> so either allocate pcie_cfg, or add a firmware_managed bool to qcom_pcie copied from pcie_cfg,
> or move runtime pm callbacks in qcom_pcie_ops and don't declare any in cfg_fw_managed->ops.
>
> I think the latter would be more scalable so we could add runtime pm variant handling
> for each IP versions. But it may be quite quite useless for now.
>
Or just bail out if dev_get_drvdata() return NULL?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-06 22:13 ` [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality Mayank Rana
2024-11-07 8:45 ` neil.armstrong
2024-11-09 23:45 ` kernel test robot
@ 2024-11-15 11:25 ` Manivannan Sadhasivam
2024-11-15 18:28 ` Mayank Rana
2 siblings, 1 reply; 24+ messages in thread
From: Manivannan Sadhasivam @ 2024-11-15 11:25 UTC (permalink / raw)
To: Mayank Rana
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On Wed, Nov 06, 2024 at 02:13:41PM -0800, Mayank Rana wrote:
> On SA8255p ride platform, PCIe root complex is firmware managed as well
> configured into ECAM compliant mode. This change adds functionality to
> enable resource management (system resource as well PCIe controller and
> PHY configuration) through firmware, and enumerating ECAM compliant root
> complex.
>
> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 1 +
> drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
> 2 files changed, 108 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index b6d6778b0698..0fe76bd39d69 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -275,6 +275,7 @@ config PCIE_QCOM
> select PCIE_DW_HOST
> select CRC8
> select PCIE_QCOM_COMMON
> + select PCI_HOST_COMMON
> help
> Say Y here to enable PCIe controller support on Qualcomm SoCs. The
> PCIe controller uses the DesignWare core plus Qualcomm-specific
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index ef44a82be058..2cb74f902baf 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -21,7 +21,9 @@
> #include <linux/limits.h>
> #include <linux/init.h>
> #include <linux/of.h>
> +#include <linux/of_pci.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/pm_opp.h>
> #include <linux/pm_runtime.h>
> #include <linux/platform_device.h>
> @@ -254,10 +256,12 @@ struct qcom_pcie_ops {
> * @ops: qcom PCIe ops structure
> * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
> * snooping
> + * @firmware_managed: Set if PCIe root complex is firmware managed
ecam_compliant?
> */
> struct qcom_pcie_cfg {
> const struct qcom_pcie_ops *ops;
> bool override_no_snoop;
> + bool firmware_managed;
> bool no_l0s;
> };
>
> @@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
> .no_l0s = true,
> };
>
> +static const struct qcom_pcie_cfg cfg_fw_managed = {
cfg_ecam?
> + .firmware_managed = true,
> +};
> +
> static const struct dw_pcie_ops dw_pcie_ops = {
> .link_up = qcom_pcie_link_up,
> .start_link = qcom_pcie_start_link,
> @@ -1566,6 +1574,51 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static void qcom_pci_free_msi(void *ptr)
> +{
> + struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
> +
> + if (pp && pp->has_msi_ctrl)
> + dw_pcie_free_msi(pp);
> +}
> +
> +static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
> +{
> + struct device *dev = cfg->parent;
> + struct dw_pcie_rp *pp;
> + struct dw_pcie *pci;
> + int ret;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> + if (!pci)
> + return -ENOMEM;
> +
> + pci->dev = dev;
> + pp = &pci->pp;
> + pci->dbi_base = cfg->win;
> + pp->num_vectors = MSI_DEF_NUM_VECTORS;
> +
> + ret = dw_pcie_msi_host_init(pp);
> + if (ret)
> + return ret;
> +
> + pp->has_msi_ctrl = true;
> + dw_pcie_msi_init(pp);
> +
> + ret = devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
return devm_add_action_or_reset()...
> + return ret;
> +}
> +
> +/* ECAM ops */
> +const struct pci_ecam_ops pci_qcom_ecam_ops = {
> + .init = qcom_pcie_ecam_host_init,
> + .pci_ops = {
> + .map_bus = pci_ecam_map_bus,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> + }
> +};
> +
> static int qcom_pcie_probe(struct platform_device *pdev)
> {
> const struct qcom_pcie_cfg *pcie_cfg;
> @@ -1580,11 +1633,52 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> char *name;
>
> pcie_cfg = of_device_get_match_data(dev);
> - if (!pcie_cfg || !pcie_cfg->ops) {
> - dev_err(dev, "Invalid platform data\n");
> + if (!pcie_cfg) {
> + dev_err(dev, "No platform data\n");
> + return -EINVAL;
> + }
> +
> + if (!pcie_cfg->firmware_managed && !pcie_cfg->ops) {
> + dev_err(dev, "No platform ops\n");
> return -EINVAL;
> }
>
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0)
> + goto err_pm_runtime_put;
> +
> + if (pcie_cfg->firmware_managed) {
> + struct pci_host_bridge *bridge;
> + struct pci_config_window *cfg;
> +
> + bridge = devm_pci_alloc_host_bridge(dev, 0);
> + if (!bridge) {
> + ret = -ENOMEM;
> + goto err_pm_runtime_put;
> + }
> +
> + of_pci_check_probe_only();
You haven't defined the "linux,pci-probe-only" property in DT. So if everything
works fine, then you can leave this call.
> + /* Parse and map our Configuration Space windows */
> + cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
> + if (IS_ERR(cfg)) {
> + ret = PTR_ERR(cfg);
> + goto err_pm_runtime_put;
> + }
> +
> + bridge->sysdata = cfg;
> + bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
> + bridge->msi_domain = true;
> +
> + ret = pci_host_probe(bridge);
return pci_host_probe()...
> + if (ret) {
> + dev_err(dev, "pci_host_probe() failed:%d\n", ret);
> + goto err_pm_runtime_put;
> + }
> +
> + return ret;
> + }
> +
> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> if (!pcie)
> return -ENOMEM;
> @@ -1593,11 +1687,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> if (!pci)
> return -ENOMEM;
>
> - pm_runtime_enable(dev);
> - ret = pm_runtime_get_sync(dev);
> - if (ret < 0)
> - goto err_pm_runtime_put;
> -
> pci->dev = dev;
> pci->ops = &dw_pcie_ops;
> pp = &pci->pp;
> @@ -1739,9 +1828,13 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>
> static int qcom_pcie_suspend_noirq(struct device *dev)
> {
> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
> + struct qcom_pcie *pcie;
> int ret = 0;
>
> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
> + return 0;
How about bailing out if dev_get_drvdata() returns NULL?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex
2024-11-06 22:13 [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Mayank Rana
` (3 preceding siblings ...)
2024-11-06 22:13 ` [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality Mayank Rana
@ 2024-11-15 11:28 ` Manivannan Sadhasivam
2024-11-15 18:31 ` Mayank Rana
4 siblings, 1 reply; 24+ messages in thread
From: Manivannan Sadhasivam @ 2024-11-15 11:28 UTC (permalink / raw)
To: Mayank Rana
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On Wed, Nov 06, 2024 at 02:13:37PM -0800, Mayank Rana wrote:
> Based on received feedback, this patch series adds support with existing
> Linux qcom-pcie.c driver to get PCIe host root complex functionality on
> Qualcomm SA8255P auto platform.
>
> 1. Interface to allow requesting firmware to manage system resources and
> performing PCIe Link up (devicetree binding in terms of power domain and
> runtime PM APIs is used in driver)
>
> 2. SA8255P is using Synopsys Designware PCIe controller which supports MSI
> controller. Using existing MSI controller based functionality by exporting
> important pcie dwc core driver based MSI APIs, and using those from
> pcie-qcom.c driver.
>
> Below architecture is used on Qualcomm SA8255P auto platform to get ECAM
> compliant PCIe controller based functionality. Here firmware VM based PCIe
> driver takes care of resource management and performing PCIe link related
> handling (D0 and D3cold). Linux pcie-qcom.c driver uses power domain to
> request firmware VM to perform these operations using SCMI interface.
> --------------------
>
>
> ┌────────────────────────┐
> │ │
> ┌──────────────────────┐ │ SHARED MEMORY │ ┌──────────────────────────┐
> │ Firmware VM │ │ │ │ Linux VM │
> │ ┌─────────┐ │ │ │ │ ┌────────────────┐ │
> │ │ Drivers │ ┌──────┐ │ │ │ │ │ PCIE Qcom │ │
> │ │ PCIE PHY◄─┤ │ │ │ ┌────────────────┐ │ │ │ driver │ │
> │ │ │ │ SCMI │ │ │ │ │ │ │ │ │ │
> │ │PCIE CTL │ │ │ ├─────────┼───► PCIE ◄───┼─────┐ │ └──┬──────────▲──┘ │
> │ │ ├─►Server│ │ │ │ SHMEM │ │ │ │ │ │ │
> │ │Clk, Vreg│ │ │ │ │ │ │ │ │ │ ┌──▼──────────┴──┐ │
> │ │GPIO,GDSC│ └─▲──┬─┘ │ │ └────────────────┘ │ └──────┼────┤PCIE SCMI Inst │ │
> │ └─────────┘ │ │ │ │ │ │ └──▲──────────┬──┘ │
> │ │ │ │ │ │ │ │ │ │
> └───────────────┼──┼───┘ │ │ └───────┼──────────┼───────┘
> │ │ │ │ │ │
> │ │ └────────────────────────┘ │ │
> │ │ │ │
> │ │ │ │
> │ │ │ │
> │ │ │IRQ │HVC
> IRQ │ │HVC │ │
> │ │ │ │
> │ │ │ │
> │ │ │ │
> ┌─────────────────┴──▼───────────────────────────────────────────────────────────┴──────────▼──────────────┐
> │ │
> │ │
> │ HYPERVISOR │
> │ │
> │ │
> │ │
> └──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
>
> ┌─────────────┐ ┌─────────────┐ ┌──────────┐ ┌───────────┐ ┌─────────────┐ ┌────────────┐
> │ │ │ │ │ │ │ │ │ PCIE │ │ PCIE │
> │ CLOCK │ │ REGULATOR │ │ GPIO │ │ GDSC │ │ PHY │ │ controller │
> └─────────────┘ └─────────────┘ └──────────┘ └───────────┘ └─────────────┘ └────────────┘
>
Thanks a lot for working on this Mayank! This version looks good to me. I've
left some comments, nothing alarming though.
But I do want to hold up this series until we finalize the SCMI based design.
- Mani
> ----------
> Changes in V3:
> - Drop usage of PCIE host generic driver usage, and splitting of MSI functionality
> - Modified existing pcie-qcom.c driver to add support for getting ECAM compliant and firmware managed
> PCIe root complex functionality
> Link to v2: https://lore.kernel.org/linux-arm-kernel/925d1eca-975f-4eec-bdf8-ca07a892361a@quicinc.com/T/
>
> Changes in V2:
> - Drop new PCIe Qcom ECAM driver, and use existing PCIe designware based MSI functionality
> - Add power domain based functionality within existing ECAM driver
> Link to v1: https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
>
> Tested:
> - Validated NVME functionality with PCIe0 on SA8255P-RIDE platform
>
> Mayank Rana (3):
> PCI: dwc: Export dwc MSI controller related APIs
> PCI: qcom: Add firmware managed ECAM compliant PCIe root complex
> functionality
> dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root
> complex
>
> .../devicetree/bindings/pci/qcom,pcie-sa8255p.yaml | 100 +++++++++++++++++++++
> drivers/pci/controller/dwc/Kconfig | 1 +
> drivers/pci/controller/dwc/pcie-designware-host.c | 38 ++++----
> drivers/pci/controller/dwc/pcie-designware.h | 14 +++
> drivers/pci/controller/dwc/pcie-qcom.c | 69 ++++++++++++--
> 5 files changed, 199 insertions(+), 23 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
>
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/4] PCI: dwc: Export dwc MSI controller related APIs
2024-11-15 9:14 ` Manivannan Sadhasivam
@ 2024-11-15 18:15 ` Mayank Rana
0 siblings, 0 replies; 24+ messages in thread
From: Mayank Rana @ 2024-11-15 18:15 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On 11/15/2024 1:14 AM, Manivannan Sadhasivam wrote:
> On Wed, Nov 06, 2024 at 02:13:38PM -0800, Mayank Rana wrote:
>> To allow dwc PCIe controller based MSI functionality from ECAM pcie
>> driver, export dw_pcie_msi_host_init(), dw_pcie_msi_init() and
>> dw_pcie_msi_free() APIs. Also move MSI IRQ related initialization code
>> into dw_pcie_msi_init() as this code executes before dw_pcie_msi_init()
>> API to use with ECAM driver.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> .../pci/controller/dwc/pcie-designware-host.c | 38 ++++++++++---------
>> drivers/pci/controller/dwc/pcie-designware.h | 14 +++++++
>> 2 files changed, 34 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 3e41865c7290..25020a090db8 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -250,7 +250,7 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
>> return 0;
>> }
>>
>> -static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
>> +void dw_pcie_free_msi(struct dw_pcie_rp *pp)
>> {
>> u32 ctrl;
>>
>> @@ -263,19 +263,34 @@ static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
>> irq_domain_remove(pp->msi_domain);
>> irq_domain_remove(pp->irq_domain);
>> }
>> +EXPORT_SYMBOL_GPL(dw_pcie_free_msi);
>>
>> -static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
>> +void dw_pcie_msi_init(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> u64 msi_target = (u64)pp->msi_data;
>> + u32 ctrl, num_ctrls;
>>
>> if (!pci_msi_enabled() || !pp->has_msi_ctrl)
>> return;
>>
>> + num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
>> +
>> + /* Initialize IRQ Status array */
>> + for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
>> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
>> + (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>> + pp->irq_mask[ctrl]);
>> + dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
>> + (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>> + ~0);
>> + }
>> +
>> /* Program the msi_data */
>> dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
>> dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
>> }
>> +EXPORT_SYMBOL_GPL(dw_pcie_msi_init);
>>
>> static int dw_pcie_parse_split_msi_irq(struct dw_pcie_rp *pp)
>> {
>> @@ -317,7 +332,7 @@ static int dw_pcie_parse_split_msi_irq(struct dw_pcie_rp *pp)
>> return 0;
>> }
>>
>> -static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
>> +int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> struct device *dev = pci->dev;
>> @@ -391,6 +406,7 @@ static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
>>
>> return 0;
>> }
>> +EXPORT_SYMBOL_GPL(dw_pcie_msi_host_init);
>>
>> static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
>> {
>> @@ -802,7 +818,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>> int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> - u32 val, ctrl, num_ctrls;
>> + u32 val;
>> int ret;
>>
>> /*
>> @@ -813,20 +829,6 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
>>
>> dw_pcie_setup(pci);
>>
>> - if (pp->has_msi_ctrl) {
>> - num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
>> -
>> - /* Initialize IRQ Status array */
>> - for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
>> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
>> - (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>> - pp->irq_mask[ctrl]);
>> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
>> - (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>> - ~0);
>> - }
>> - }
>> -
>> dw_pcie_msi_init(pp);
>>
>> /* Setup RC BARs */
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 347ab74ac35a..ef748d82c663 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -679,6 +679,9 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
>>
>> #ifdef CONFIG_PCIE_DW_HOST
>> irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
>> +void dw_pcie_msi_init(struct dw_pcie_rp *pp);
>> +int dw_pcie_msi_host_init(struct dw_pcie_rp *pp);
>> +void dw_pcie_free_msi(struct dw_pcie_rp *pp);
>> int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
>> int dw_pcie_host_init(struct dw_pcie_rp *pp);
>> void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
>> @@ -691,6 +694,17 @@ static inline irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp)
>> return IRQ_NONE;
>> }
>>
>> +static void dw_pcie_msi_init(struct dw_pcie_rp *pp)
>
> Missing 'inline' here and below?
ACK
>
> - Mani
>
>> +{ }
>> +
>> +static int dw_pcie_msi_host_init(struct dw_pcie_rp *pp)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> +static void dw_pcie_free_msi(struct dw_pcie_rp *pp)
>> +{ }
>> +
>> static inline int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
>> {
>> return 0;
>> --
>> 2.25.1
>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/4] PCI: host-generic: Export gen_pci_init() API to allow ECAM creation
2024-11-15 9:17 ` Manivannan Sadhasivam
@ 2024-11-15 18:16 ` Mayank Rana
0 siblings, 0 replies; 24+ messages in thread
From: Mayank Rana @ 2024-11-15 18:16 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On 11/15/2024 1:17 AM, Manivannan Sadhasivam wrote:
> On Wed, Nov 06, 2024 at 02:13:39PM -0800, Mayank Rana wrote:
>> Export gen_pci_init() API to create ECAM and initialized ECAM OPs
>> from PCIe driver which don't have way to populate driver_data as
>> just ECAM ops.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> drivers/pci/controller/pci-host-common.c | 3 ++-
>> include/linux/pci-ecam.h | 2 ++
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
>> index cf5f59a745b3..b9460a4c5b7e 100644
>> --- a/drivers/pci/controller/pci-host-common.c
>> +++ b/drivers/pci/controller/pci-host-common.c
>> @@ -20,7 +20,7 @@ static void gen_pci_unmap_cfg(void *ptr)
>> pci_ecam_free((struct pci_config_window *)ptr);
>> }
>>
>> -static struct pci_config_window *gen_pci_init(struct device *dev,
>> +struct pci_config_window *gen_pci_init(struct device *dev,
>
> Please rename the API to something like 'pci_host_common_init()'.
> 'gen_pci_init()' is fine within the driver, but doesn't look good when exported
> outside.
ACK
> - Mani
>
>> struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops)
>> {
>> int err;
>> @@ -48,6 +48,7 @@ static struct pci_config_window *gen_pci_init(struct device *dev,
>>
>> return cfg;
>> }
>> +EXPORT_SYMBOL_GPL(gen_pci_init);
>>
>> int pci_host_common_probe(struct platform_device *pdev)
>> {
>> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
>> index 3a4860bd2758..386c08349169 100644
>> --- a/include/linux/pci-ecam.h
>> +++ b/include/linux/pci-ecam.h
>> @@ -94,5 +94,7 @@ extern const struct pci_ecam_ops loongson_pci_ecam_ops; /* Loongson PCIe */
>> /* for DT-based PCI controllers that support ECAM */
>> int pci_host_common_probe(struct platform_device *pdev);
>> void pci_host_common_remove(struct platform_device *pdev);
>> +struct pci_config_window *gen_pci_init(struct device *dev,
>> + struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
>> #endif
>> #endif
>> --
>> 2.25.1
>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-15 11:21 ` Manivannan Sadhasivam
@ 2024-11-15 18:17 ` Mayank Rana
0 siblings, 0 replies; 24+ messages in thread
From: Mayank Rana @ 2024-11-15 18:17 UTC (permalink / raw)
To: Manivannan Sadhasivam, neil.armstrong
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On 11/15/2024 3:21 AM, Manivannan Sadhasivam wrote:
> On Fri, Nov 08, 2024 at 11:22:52AM +0100, neil.armstrong@linaro.org wrote:
>> On 07/11/2024 18:45, Mayank Rana wrote:
>>>
>>>
>>> On 11/7/2024 12:45 AM, neil.armstrong@linaro.org wrote:
>>>> Hi,
>>>>
>>>> On 06/11/2024 23:13, Mayank Rana wrote:
>>>>> On SA8255p ride platform, PCIe root complex is firmware managed as well
>>>>> configured into ECAM compliant mode. This change adds functionality to
>>>>> enable resource management (system resource as well PCIe controller and
>>>>> PHY configuration) through firmware, and enumerating ECAM compliant root
>>>>> complex.
>>>>>
>>>>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>>>>> ---
>>>>> drivers/pci/controller/dwc/Kconfig | 1 +
>>>>> drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
>>>>> 2 files changed, 108 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>>>>> index b6d6778b0698..0fe76bd39d69 100644
>>>>> --- a/drivers/pci/controller/dwc/Kconfig
>>>>> +++ b/drivers/pci/controller/dwc/Kconfig
>>>>> @@ -275,6 +275,7 @@ config PCIE_QCOM
>>>>> select PCIE_DW_HOST
>>>>> select CRC8
>>>>> select PCIE_QCOM_COMMON
>>>>> + select PCI_HOST_COMMON
>>>>> help
>>>>> Say Y here to enable PCIe controller support on Qualcomm SoCs. The
>>>>> PCIe controller uses the DesignWare core plus Qualcomm-specific
>>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> index ef44a82be058..2cb74f902baf 100644
>>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> @@ -21,7 +21,9 @@
>>>>> #include <linux/limits.h>
>>>>> #include <linux/init.h>
>>>>> #include <linux/of.h>
>>>>> +#include <linux/of_pci.h>
>>>>> #include <linux/pci.h>
>>>>> +#include <linux/pci-ecam.h>
>>>>> #include <linux/pm_opp.h>
>>>>> #include <linux/pm_runtime.h>
>>>>> #include <linux/platform_device.h>
>>>>> @@ -254,10 +256,12 @@ struct qcom_pcie_ops {
>>>>> * @ops: qcom PCIe ops structure
>>>>> * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
>>>>> * snooping
>>>>> + * @firmware_managed: Set if PCIe root complex is firmware managed
>>>>> */
>>>>> struct qcom_pcie_cfg {
>>>>> const struct qcom_pcie_ops *ops;
>>>>> bool override_no_snoop;
>>>>> + bool firmware_managed;
>>>>> bool no_l0s;
>>>>> };
>>>>> @@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>>>>> .no_l0s = true,
>>>>> };
>>>>> +static const struct qcom_pcie_cfg cfg_fw_managed = {
>>>>> + .firmware_managed = true,
>>>>> +};
>>>>> +
>>>>> static const struct dw_pcie_ops dw_pcie_ops = {
>>>>> .link_up = qcom_pcie_link_up,
>>>>> .start_link = qcom_pcie_start_link,
>>>>> @@ -1566,6 +1574,51 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
>>>>> return IRQ_HANDLED;
>>>>> }
>>>>> +static void qcom_pci_free_msi(void *ptr)
>>>>> +{
>>>>> + struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
>>>>> +
>>>>> + if (pp && pp->has_msi_ctrl)
>>>>> + dw_pcie_free_msi(pp);
>>>>> +}
>>>>> +
>>>>> +static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
>>>>> +{
>>>>> + struct device *dev = cfg->parent;
>>>>> + struct dw_pcie_rp *pp;
>>>>> + struct dw_pcie *pci;
>>>>> + int ret;
>>>>> +
>>>>> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>>>>> + if (!pci)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + pci->dev = dev;
>>>>> + pp = &pci->pp;
>>>>> + pci->dbi_base = cfg->win;
>>>>> + pp->num_vectors = MSI_DEF_NUM_VECTORS;
>>>>> +
>>>>> + ret = dw_pcie_msi_host_init(pp);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + pp->has_msi_ctrl = true;
>>>>> + dw_pcie_msi_init(pp);
>>>>> +
>>>>> + ret = devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +/* ECAM ops */
>>>>> +const struct pci_ecam_ops pci_qcom_ecam_ops = {
>>>>> + .init = qcom_pcie_ecam_host_init,
>>>>> + .pci_ops = {
>>>>> + .map_bus = pci_ecam_map_bus,
>>>>> + .read = pci_generic_config_read,
>>>>> + .write = pci_generic_config_write,
>>>>> + }
>>>>> +};
>>>>> +
>>>>> static int qcom_pcie_probe(struct platform_device *pdev)
>>>>> {
>>>>> const struct qcom_pcie_cfg *pcie_cfg;
>>>>> @@ -1580,11 +1633,52 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>>>> char *name;
>>>>> pcie_cfg = of_device_get_match_data(dev);
>>>>> - if (!pcie_cfg || !pcie_cfg->ops) {
>>>>> - dev_err(dev, "Invalid platform data\n");
>>>>> + if (!pcie_cfg) {
>>>>> + dev_err(dev, "No platform data\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + if (!pcie_cfg->firmware_managed && !pcie_cfg->ops) {
>>>>> + dev_err(dev, "No platform ops\n");
>>>>> return -EINVAL;
>>>>> }
>>>>> + pm_runtime_enable(dev);
>>>>> + ret = pm_runtime_get_sync(dev);
>>>>> + if (ret < 0)
>>>>> + goto err_pm_runtime_put;
>>>>> +
>>>>> + if (pcie_cfg->firmware_managed) {
>>>>> + struct pci_host_bridge *bridge;
>>>>> + struct pci_config_window *cfg;
>>>>> +
>>>>> + bridge = devm_pci_alloc_host_bridge(dev, 0);
>>>>> + if (!bridge) {
>>>>> + ret = -ENOMEM;
>>>>> + goto err_pm_runtime_put;
>>>>> + }
>>>>> +
>>>>> + of_pci_check_probe_only();
>>>>> + /* Parse and map our Configuration Space windows */
>>>>> + cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
>>>>> + if (IS_ERR(cfg)) {
>>>>> + ret = PTR_ERR(cfg);
>>>>> + goto err_pm_runtime_put;
>>>>> + }
>>>>> +
>>>>> + bridge->sysdata = cfg;
>>>>> + bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
>>>>> + bridge->msi_domain = true;
>>>>> +
>>>>> + ret = pci_host_probe(bridge);
>>>>> + if (ret) {
>>>>> + dev_err(dev, "pci_host_probe() failed:%d\n", ret);
>>>>> + goto err_pm_runtime_put;
>>>>> + }
>>>>> +
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>>>>> if (!pcie)
>>>>> return -ENOMEM;
>>>>> @@ -1593,11 +1687,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>>>> if (!pci)
>>>>> return -ENOMEM;
>>>>> - pm_runtime_enable(dev);
>>>>> - ret = pm_runtime_get_sync(dev);
>>>>> - if (ret < 0)
>>>>> - goto err_pm_runtime_put;
>>>>> -
>>>>> pci->dev = dev;
>>>>> pci->ops = &dw_pcie_ops;
>>>>> pp = &pci->pp;
>>>>> @@ -1739,9 +1828,13 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>>>> static int qcom_pcie_suspend_noirq(struct device *dev)
>>>>> {
>>>>> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
>>>>> + struct qcom_pcie *pcie;
>>>>> int ret = 0;
>>>>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
>>>>
>>>> Can't you use if (pcie_cfg->firmware_managed) here instead ?
>>> yes, although with firmware managed mode, struct qcom_pcie *pcie is not allocated, and just
>>> to get access to pcie_cfg for this check, I took this approach. I am thiking to do allocating struct qcom_pcie *pcie and using it in future if we need more other related functionality which needs usage of this structure for functionality like global interrupt etc.
>>>
>>> Although if you still prefer to allocate struct qcom_pcie based memory to access pcie_cfg, then I can consider to update in next patchset. Please suggest.
>>
>> I understand, but running of_device_is_compatible() in runtime PM is not something we should do,
>> so either allocate pcie_cfg, or add a firmware_managed bool to qcom_pcie copied from pcie_cfg,
>> or move runtime pm callbacks in qcom_pcie_ops and don't declare any in cfg_fw_managed->ops.
>>
>> I think the latter would be more scalable so we could add runtime pm variant handling
>> for each IP versions. But it may be quite quite useless for now.
>>
>
> Or just bail out if dev_get_drvdata() return NULL?
Agree. This would also work for current requirement.
Regards,
Mayank
>
> - Mani
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-15 11:25 ` Manivannan Sadhasivam
@ 2024-11-15 18:28 ` Mayank Rana
2024-11-19 17:14 ` Manivannan Sadhasivam
0 siblings, 1 reply; 24+ messages in thread
From: Mayank Rana @ 2024-11-15 18:28 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On 11/15/2024 3:25 AM, Manivannan Sadhasivam wrote:
> On Wed, Nov 06, 2024 at 02:13:41PM -0800, Mayank Rana wrote:
>> On SA8255p ride platform, PCIe root complex is firmware managed as well
>> configured into ECAM compliant mode. This change adds functionality to
>> enable resource management (system resource as well PCIe controller and
>> PHY configuration) through firmware, and enumerating ECAM compliant root
>> complex.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> drivers/pci/controller/dwc/Kconfig | 1 +
>> drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
>> 2 files changed, 108 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index b6d6778b0698..0fe76bd39d69 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -275,6 +275,7 @@ config PCIE_QCOM
>> select PCIE_DW_HOST
>> select CRC8
>> select PCIE_QCOM_COMMON
>> + select PCI_HOST_COMMON
>> help
>> Say Y here to enable PCIe controller support on Qualcomm SoCs. The
>> PCIe controller uses the DesignWare core plus Qualcomm-specific
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index ef44a82be058..2cb74f902baf 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -21,7 +21,9 @@
>> #include <linux/limits.h>
>> #include <linux/init.h>
>> #include <linux/of.h>
>> +#include <linux/of_pci.h>
>> #include <linux/pci.h>
>> +#include <linux/pci-ecam.h>
>> #include <linux/pm_opp.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/platform_device.h>
>> @@ -254,10 +256,12 @@ struct qcom_pcie_ops {
>> * @ops: qcom PCIe ops structure
>> * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
>> * snooping
>> + * @firmware_managed: Set if PCIe root complex is firmware managed
>
> ecam_compliant?
I assume you mean to update as Set if ECAM compliant PCIe root complex
is firmware manage
>> */
>> struct qcom_pcie_cfg {
>> const struct qcom_pcie_ops *ops;
>> bool override_no_snoop;
>> + bool firmware_managed;
>> bool no_l0s;
>> };
>>
>> @@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>> .no_l0s = true,
>> };
>>
>> +static const struct qcom_pcie_cfg cfg_fw_managed = {
>
> cfg_ecam?
Putting more emphasize on fw_managed as don't want to conflict with new
work in progress (krishna is working on it)
to make Qualcomm PCIe root complex as ECAM compliant (non firmware
managed one). are you OK with using cfg_ecam_fw_managed ?
>> + .firmware_managed = true,
>> +};
>> +
>> static const struct dw_pcie_ops dw_pcie_ops = {
>> .link_up = qcom_pcie_link_up,
>> .start_link = qcom_pcie_start_link,
>> @@ -1566,6 +1574,51 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
>> return IRQ_HANDLED;
>> }
>>
>> +static void qcom_pci_free_msi(void *ptr)
>> +{
>> + struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
>> +
>> + if (pp && pp->has_msi_ctrl)
>> + dw_pcie_free_msi(pp);
>> +}
>> +
>> +static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
>> +{
>> + struct device *dev = cfg->parent;
>> + struct dw_pcie_rp *pp;
>> + struct dw_pcie *pci;
>> + int ret;
>> +
>> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>> + if (!pci)
>> + return -ENOMEM;
>> +
>> + pci->dev = dev;
>> + pp = &pci->pp;
>> + pci->dbi_base = cfg->win;
>> + pp->num_vectors = MSI_DEF_NUM_VECTORS;
>> +
>> + ret = dw_pcie_msi_host_init(pp);
>> + if (ret)
>> + return ret;
>> +
>> + pp->has_msi_ctrl = true;
>> + dw_pcie_msi_init(pp);
>> +
>> + ret = devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
>
> return devm_add_action_or_reset()...
Done.
>> + return ret;
>> +}
>> +
>> +/* ECAM ops */
>> +const struct pci_ecam_ops pci_qcom_ecam_ops = {
>> + .init = qcom_pcie_ecam_host_init,
>> + .pci_ops = {
>> + .map_bus = pci_ecam_map_bus,
>> + .read = pci_generic_config_read,
>> + .write = pci_generic_config_write,
>> + }
>> +};
>> +
>> static int qcom_pcie_probe(struct platform_device *pdev)
>> {
>> const struct qcom_pcie_cfg *pcie_cfg;
>> @@ -1580,11 +1633,52 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>> char *name;
>>
>> pcie_cfg = of_device_get_match_data(dev);
>> - if (!pcie_cfg || !pcie_cfg->ops) {
>> - dev_err(dev, "Invalid platform data\n");
>> + if (!pcie_cfg) {
>> + dev_err(dev, "No platform data\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (!pcie_cfg->firmware_managed && !pcie_cfg->ops) {
>> + dev_err(dev, "No platform ops\n");
>> return -EINVAL;
>> }
>>
>> + pm_runtime_enable(dev);
>> + ret = pm_runtime_get_sync(dev);
>> + if (ret < 0)
>> + goto err_pm_runtime_put;
>> +
>> + if (pcie_cfg->firmware_managed) {
>> + struct pci_host_bridge *bridge;
>> + struct pci_config_window *cfg;
>> +
>> + bridge = devm_pci_alloc_host_bridge(dev, 0);
>> + if (!bridge) {
>> + ret = -ENOMEM;
>> + goto err_pm_runtime_put;
>> + }
>> +
>> + of_pci_check_probe_only();
>
> You haven't defined the "linux,pci-probe-only" property in DT. So if everything
> works fine, then you can leave this call.
ok will review more and update accordingly.
>> + /* Parse and map our Configuration Space windows */
>> + cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
>> + if (IS_ERR(cfg)) {
>> + ret = PTR_ERR(cfg);
>> + goto err_pm_runtime_put;
>> + }
>> +
>> + bridge->sysdata = cfg;
>> + bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
>> + bridge->msi_domain = true;
>> +
>> + ret = pci_host_probe(bridge);
>
> return pci_host_probe()...
need to perform pm_runtime_put_sync() and pm_runtime_disable() in
failure case.
Hence checking error here and doing goto err_pm_runtime_put
>> + if (ret) {
>> + dev_err(dev, "pci_host_probe() failed:%d\n", ret);
>> + goto err_pm_runtime_put;
>> + }
>> +
>> + return ret;
>> + }
>> +
>> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>> if (!pcie)
>> return -ENOMEM;
>> @@ -1593,11 +1687,6 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>> if (!pci)
>> return -ENOMEM;
>>
>> - pm_runtime_enable(dev);
>> - ret = pm_runtime_get_sync(dev);
>> - if (ret < 0)
>> - goto err_pm_runtime_put;
>> -
>> pci->dev = dev;
>> pci->ops = &dw_pcie_ops;
>> pp = &pci->pp;
>> @@ -1739,9 +1828,13 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>
>> static int qcom_pcie_suspend_noirq(struct device *dev)
>> {
>> - struct qcom_pcie *pcie = dev_get_drvdata(dev);
>> + struct qcom_pcie *pcie;
>> int ret = 0;
>>
>> + if (of_device_is_compatible(dev->of_node, "qcom,pcie-sa8255p"))
>> + return 0;
>
> How about bailing out if dev_get_drvdata() returns NULL?
Done
Regards,
Mayank
> - Mani
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex
2024-11-15 11:28 ` [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Manivannan Sadhasivam
@ 2024-11-15 18:31 ` Mayank Rana
2024-11-19 17:10 ` Manivannan Sadhasivam
0 siblings, 1 reply; 24+ messages in thread
From: Mayank Rana @ 2024-11-15 18:31 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On 11/15/2024 3:28 AM, Manivannan Sadhasivam wrote:
> On Wed, Nov 06, 2024 at 02:13:37PM -0800, Mayank Rana wrote:
>> Based on received feedback, this patch series adds support with existing
>> Linux qcom-pcie.c driver to get PCIe host root complex functionality on
>> Qualcomm SA8255P auto platform.
>>
>> 1. Interface to allow requesting firmware to manage system resources and
>> performing PCIe Link up (devicetree binding in terms of power domain and
>> runtime PM APIs is used in driver)
>>
>> 2. SA8255P is using Synopsys Designware PCIe controller which supports MSI
>> controller. Using existing MSI controller based functionality by exporting
>> important pcie dwc core driver based MSI APIs, and using those from
>> pcie-qcom.c driver.
>>
>> Below architecture is used on Qualcomm SA8255P auto platform to get ECAM
>> compliant PCIe controller based functionality. Here firmware VM based PCIe
>> driver takes care of resource management and performing PCIe link related
>> handling (D0 and D3cold). Linux pcie-qcom.c driver uses power domain to
>> request firmware VM to perform these operations using SCMI interface.
>> --------------------
>>
>>
>> ┌────────────────────────┐
>> │ │
>> ┌──────────────────────┐ │ SHARED MEMORY │ ┌──────────────────────────┐
>> │ Firmware VM │ │ │ │ Linux VM │
>> │ ┌─────────┐ │ │ │ │ ┌────────────────┐ │
>> │ │ Drivers │ ┌──────┐ │ │ │ │ │ PCIE Qcom │ │
>> │ │ PCIE PHY◄─┤ │ │ │ ┌────────────────┐ │ │ │ driver │ │
>> │ │ │ │ SCMI │ │ │ │ │ │ │ │ │ │
>> │ │PCIE CTL │ │ │ ├─────────┼───► PCIE ◄───┼─────┐ │ └──┬──────────▲──┘ │
>> │ │ ├─►Server│ │ │ │ SHMEM │ │ │ │ │ │ │
>> │ │Clk, Vreg│ │ │ │ │ │ │ │ │ │ ┌──▼──────────┴──┐ │
>> │ │GPIO,GDSC│ └─▲──┬─┘ │ │ └────────────────┘ │ └──────┼────┤PCIE SCMI Inst │ │
>> │ └─────────┘ │ │ │ │ │ │ └──▲──────────┬──┘ │
>> │ │ │ │ │ │ │ │ │ │
>> └───────────────┼──┼───┘ │ │ └───────┼──────────┼───────┘
>> │ │ │ │ │ │
>> │ │ └────────────────────────┘ │ │
>> │ │ │ │
>> │ │ │ │
>> │ │ │ │
>> │ │ │IRQ │HVC
>> IRQ │ │HVC │ │
>> │ │ │ │
>> │ │ │ │
>> │ │ │ │
>> ┌─────────────────┴──▼───────────────────────────────────────────────────────────┴──────────▼──────────────┐
>> │ │
>> │ │
>> │ HYPERVISOR │
>> │ │
>> │ │
>> │ │
>> └──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
>>
>> ┌─────────────┐ ┌─────────────┐ ┌──────────┐ ┌───────────┐ ┌─────────────┐ ┌────────────┐
>> │ │ │ │ │ │ │ │ │ PCIE │ │ PCIE │
>> │ CLOCK │ │ REGULATOR │ │ GPIO │ │ GDSC │ │ PHY │ │ controller │
>> └─────────────┘ └─────────────┘ └──────────┘ └───────────┘ └─────────────┘ └────────────┘
>>
>
> Thanks a lot for working on this Mayank! This version looks good to me. I've
> left some comments, nothing alarming though.
Thanks for reviewing change. I would address those in next patchset.
> But I do want to hold up this series until we finalize the SCMI based design.
ok. I want to send these changes which are prepared based on previously
provided feedback, to see if we have any major concern here in terms of
getting functionality.
Regards,
Mayank
> - Mani
>
>> ----------
>> Changes in V3:
>> - Drop usage of PCIE host generic driver usage, and splitting of MSI functionality
>> - Modified existing pcie-qcom.c driver to add support for getting ECAM compliant and firmware managed
>> PCIe root complex functionality
>> Link to v2: https://lore.kernel.org/linux-arm-kernel/925d1eca-975f-4eec-bdf8-ca07a892361a@quicinc.com/T/
>>
>> Changes in V2:
>> - Drop new PCIe Qcom ECAM driver, and use existing PCIe designware based MSI functionality
>> - Add power domain based functionality within existing ECAM driver
>> Link to v1: https://lore.kernel.org/all/d10199df-5fb3-407b-b404-a0a4d067341f@quicinc.com/T/
>>
>> Tested:
>> - Validated NVME functionality with PCIe0 on SA8255P-RIDE platform
>>
>> Mayank Rana (3):
>> PCI: dwc: Export dwc MSI controller related APIs
>> PCI: qcom: Add firmware managed ECAM compliant PCIe root complex
>> functionality
>> dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root
>> complex
>>
>> .../devicetree/bindings/pci/qcom,pcie-sa8255p.yaml | 100 +++++++++++++++++++++
>> drivers/pci/controller/dwc/Kconfig | 1 +
>> drivers/pci/controller/dwc/pcie-designware-host.c | 38 ++++----
>> drivers/pci/controller/dwc/pcie-designware.h | 14 +++
>> drivers/pci/controller/dwc/pcie-qcom.c | 69 ++++++++++++--
>> 5 files changed, 199 insertions(+), 23 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie-sa8255p.yaml
>>
>> --
>> 2.7.4
>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex
2024-11-15 18:31 ` Mayank Rana
@ 2024-11-19 17:10 ` Manivannan Sadhasivam
0 siblings, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2024-11-19 17:10 UTC (permalink / raw)
To: Mayank Rana
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On Fri, Nov 15, 2024 at 10:31:17AM -0800, Mayank Rana wrote:
>
>
> On 11/15/2024 3:28 AM, Manivannan Sadhasivam wrote:
> > On Wed, Nov 06, 2024 at 02:13:37PM -0800, Mayank Rana wrote:
> > > Based on received feedback, this patch series adds support with existing
> > > Linux qcom-pcie.c driver to get PCIe host root complex functionality on
> > > Qualcomm SA8255P auto platform.
> > >
> > > 1. Interface to allow requesting firmware to manage system resources and
> > > performing PCIe Link up (devicetree binding in terms of power domain and
> > > runtime PM APIs is used in driver)
> > >
> > > 2. SA8255P is using Synopsys Designware PCIe controller which supports MSI
> > > controller. Using existing MSI controller based functionality by exporting
> > > important pcie dwc core driver based MSI APIs, and using those from
> > > pcie-qcom.c driver.
> > >
> > > Below architecture is used on Qualcomm SA8255P auto platform to get ECAM
> > > compliant PCIe controller based functionality. Here firmware VM based PCIe
> > > driver takes care of resource management and performing PCIe link related
> > > handling (D0 and D3cold). Linux pcie-qcom.c driver uses power domain to
> > > request firmware VM to perform these operations using SCMI interface.
> > > --------------------
> > >
> > >
> > > ┌────────────────────────┐
> > > │ │
> > > ┌──────────────────────┐ │ SHARED MEMORY │ ┌──────────────────────────┐
> > > │ Firmware VM │ │ │ │ Linux VM │
> > > │ ┌─────────┐ │ │ │ │ ┌────────────────┐ │
> > > │ │ Drivers │ ┌──────┐ │ │ │ │ │ PCIE Qcom │ │
> > > │ │ PCIE PHY◄─┤ │ │ │ ┌────────────────┐ │ │ │ driver │ │
> > > │ │ │ │ SCMI │ │ │ │ │ │ │ │ │ │
> > > │ │PCIE CTL │ │ │ ├─────────┼───► PCIE ◄───┼─────┐ │ └──┬──────────▲──┘ │
> > > │ │ ├─►Server│ │ │ │ SHMEM │ │ │ │ │ │ │
> > > │ │Clk, Vreg│ │ │ │ │ │ │ │ │ │ ┌──▼──────────┴──┐ │
> > > │ │GPIO,GDSC│ └─▲──┬─┘ │ │ └────────────────┘ │ └──────┼────┤PCIE SCMI Inst │ │
> > > │ └─────────┘ │ │ │ │ │ │ └──▲──────────┬──┘ │
> > > │ │ │ │ │ │ │ │ │ │
> > > └───────────────┼──┼───┘ │ │ └───────┼──────────┼───────┘
> > > │ │ │ │ │ │
> > > │ │ └────────────────────────┘ │ │
> > > │ │ │ │
> > > │ │ │ │
> > > │ │ │ │
> > > │ │ │IRQ │HVC
> > > IRQ │ │HVC │ │
> > > │ │ │ │
> > > │ │ │ │
> > > │ │ │ │
> > > ┌─────────────────┴──▼───────────────────────────────────────────────────────────┴──────────▼──────────────┐
> > > │ │
> > > │ │
> > > │ HYPERVISOR │
> > > │ │
> > > │ │
> > > │ │
> > > └──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
> > > ┌─────────────┐ ┌─────────────┐ ┌──────────┐ ┌───────────┐ ┌─────────────┐ ┌────────────┐
> > > │ │ │ │ │ │ │ │ │ PCIE │ │ PCIE │
> > > │ CLOCK │ │ REGULATOR │ │ GPIO │ │ GDSC │ │ PHY │ │ controller │
> > > └─────────────┘ └─────────────┘ └──────────┘ └───────────┘ └─────────────┘ └────────────┘
> >
> > Thanks a lot for working on this Mayank! This version looks good to me. I've
> > left some comments, nothing alarming though.
> Thanks for reviewing change. I would address those in next patchset.
>
> > But I do want to hold up this series until we finalize the SCMI based design.
> ok. I want to send these changes which are prepared based on previously
> provided feedback, to see if we have any major concern here in terms of
> getting functionality.
>
That's fine. My comment was a note to the maintainers.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality
2024-11-15 18:28 ` Mayank Rana
@ 2024-11-19 17:14 ` Manivannan Sadhasivam
0 siblings, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2024-11-19 17:14 UTC (permalink / raw)
To: Mayank Rana
Cc: jingoohan1, will, lpieralisi, kw, robh, bhelgaas, krzk, linux-pci,
linux-arm-msm, linux-kernel, quic_krichai
On Fri, Nov 15, 2024 at 10:28:23AM -0800, Mayank Rana wrote:
>
>
> On 11/15/2024 3:25 AM, Manivannan Sadhasivam wrote:
> > On Wed, Nov 06, 2024 at 02:13:41PM -0800, Mayank Rana wrote:
> > > On SA8255p ride platform, PCIe root complex is firmware managed as well
> > > configured into ECAM compliant mode. This change adds functionality to
> > > enable resource management (system resource as well PCIe controller and
> > > PHY configuration) through firmware, and enumerating ECAM compliant root
> > > complex.
> > >
> > > Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
> > > ---
> > > drivers/pci/controller/dwc/Kconfig | 1 +
> > > drivers/pci/controller/dwc/pcie-qcom.c | 116 +++++++++++++++++++++++--
> > > 2 files changed, 108 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > index b6d6778b0698..0fe76bd39d69 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -275,6 +275,7 @@ config PCIE_QCOM
> > > select PCIE_DW_HOST
> > > select CRC8
> > > select PCIE_QCOM_COMMON
> > > + select PCI_HOST_COMMON
> > > help
> > > Say Y here to enable PCIe controller support on Qualcomm SoCs. The
> > > PCIe controller uses the DesignWare core plus Qualcomm-specific
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index ef44a82be058..2cb74f902baf 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -21,7 +21,9 @@
> > > #include <linux/limits.h>
> > > #include <linux/init.h>
> > > #include <linux/of.h>
> > > +#include <linux/of_pci.h>
> > > #include <linux/pci.h>
> > > +#include <linux/pci-ecam.h>
> > > #include <linux/pm_opp.h>
> > > #include <linux/pm_runtime.h>
> > > #include <linux/platform_device.h>
> > > @@ -254,10 +256,12 @@ struct qcom_pcie_ops {
> > > * @ops: qcom PCIe ops structure
> > > * @override_no_snoop: Override NO_SNOOP attribute in TLP to enable cache
> > > * snooping
> > > + * @firmware_managed: Set if PCIe root complex is firmware managed
> >
> > ecam_compliant?
> I assume you mean to update as Set if ECAM compliant PCIe root complex is
> firmware manage
> > > */
> > > struct qcom_pcie_cfg {
> > > const struct qcom_pcie_ops *ops;
> > > bool override_no_snoop;
> > > + bool firmware_managed;
> > > bool no_l0s;
> > > };
> > > @@ -1415,6 +1419,10 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
> > > .no_l0s = true,
> > > };
> > > +static const struct qcom_pcie_cfg cfg_fw_managed = {
> >
> > cfg_ecam?
> Putting more emphasize on fw_managed as don't want to conflict with new work
> in progress (krishna is working on it)
> to make Qualcomm PCIe root complex as ECAM compliant (non firmware managed
> one). are you OK with using cfg_ecam_fw_managed ?
>
Ah, I completely missed that. Ignore my comments about renaming to ecam.
> > > + .firmware_managed = true,
> > > +};
> > > +
[...]
> > > + /* Parse and map our Configuration Space windows */
> > > + cfg = gen_pci_init(dev, bridge, &pci_qcom_ecam_ops);
> > > + if (IS_ERR(cfg)) {
> > > + ret = PTR_ERR(cfg);
> > > + goto err_pm_runtime_put;
> > > + }
> > > +
> > > + bridge->sysdata = cfg;
> > > + bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
> > > + bridge->msi_domain = true;
> > > +
> > > + ret = pci_host_probe(bridge);
> >
> > return pci_host_probe()...
> need to perform pm_runtime_put_sync() and pm_runtime_disable() in failure
> case.
> Hence checking error here and doing goto err_pm_runtime_put
Right. This one I overlooked.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2024-11-19 17:14 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-06 22:13 [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Mayank Rana
2024-11-06 22:13 ` [PATCH v3 1/4] PCI: dwc: Export dwc MSI controller related APIs Mayank Rana
2024-11-15 9:14 ` Manivannan Sadhasivam
2024-11-15 18:15 ` Mayank Rana
2024-11-06 22:13 ` [PATCH v3 2/4] PCI: host-generic: Export gen_pci_init() API to allow ECAM creation Mayank Rana
2024-11-15 9:17 ` Manivannan Sadhasivam
2024-11-15 18:16 ` Mayank Rana
2024-11-06 22:13 ` [PATCH v3 3/4] dt-bindings: PCI: qcom,pcie-sa8255p: Document ECAM compliant PCIe root complex Mayank Rana
2024-11-07 9:35 ` Krzysztof Kozlowski
2024-11-07 17:39 ` Mayank Rana
2024-11-15 10:55 ` Manivannan Sadhasivam
2024-11-06 22:13 ` [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root complex functionality Mayank Rana
2024-11-07 8:45 ` neil.armstrong
2024-11-07 17:45 ` Mayank Rana
2024-11-08 10:22 ` neil.armstrong
2024-11-15 11:21 ` Manivannan Sadhasivam
2024-11-15 18:17 ` Mayank Rana
2024-11-09 23:45 ` kernel test robot
2024-11-15 11:25 ` Manivannan Sadhasivam
2024-11-15 18:28 ` Mayank Rana
2024-11-19 17:14 ` Manivannan Sadhasivam
2024-11-15 11:28 ` [PATCH v3 0/4] Add Qualcomm SA8255p based firmware managed PCIe root complex Manivannan Sadhasivam
2024-11-15 18:31 ` Mayank Rana
2024-11-19 17:10 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox