| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241115112504.anaatuqitdvjubyx@thinkpad>
Date: Fri, 15 Nov 2024 16:55:04 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Mayank Rana <quic_mrana@...cinc.com>
Cc: jingoohan1@...il.com, will@...nel.org, lpieralisi@...nel.org,
kw@...ux.com, robh@...nel.org, bhelgaas@...gle.com, krzk@...nel.org,
linux-pci@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, quic_krichai@...cinc.com
Subject: Re: [PATCH v3 4/4] PCI: qcom: Add Qualcomm SA8255p based PCIe root
complex functionality
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@...cinc.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
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists