[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d12e002b-e99e-4963-a732-4873e13c5419@oss.qualcomm.com>
Date: Mon, 15 Sep 2025 12:48:06 +0530
From: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
To: Bjorn Helgaas <helgaas@...nel.org>,
Manivannan Sadhasivam <mani@...nel.org>,
Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
Cc: cros-qcom-dts-watchers@...omium.org,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof WilczyĆski <kwilczynski@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Jingoo Han <jingoohan1@...il.com>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
quic_vbadigan@...cinc.com, quic_mrana@...cinc.com,
quic_vpernami@...cinc.com, mmareddy@...cinc.com
Subject: Re: [PATCH v8 5/5] PCI: qcom: Add support for ECAM feature
On 9/13/2025 2:37 AM, Bjorn Helgaas wrote:
> On Wed, Sep 03, 2025 at 02:57:21PM -0500, Bjorn Helgaas wrote:
>> On Thu, Aug 28, 2025 at 01:04:26PM +0530, Krishna Chaitanya Chundru wrote:
>>> The ELBI registers falls after the DBI space, PARF_SLV_DBI_ELBI register
>>> gives us the offset from which ELBI starts. So override ELBI with the
>>> offset from PARF_SLV_DBI_ELBI and cfg win to map these regions.
>>>
>>> On root bus, we have only the root port. Any access other than that
>>> should not go out of the link and should return all F's. Since the iATU
>>> is configured for the buses which starts after root bus, block the
>>> transactions starting from function 1 of the root bus to the end of
>>> the root bus (i.e from dbi_base + 4kb to dbi_base + 1MB) from going
>>> outside the link through ECAM blocker through PARF registers.
>
>>> +static void qcom_pci_config_ecam(struct dw_pcie_rp *pp)
>>> +{
>>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>> + struct qcom_pcie *pcie = to_qcom_pcie(pci);
>>> + u64 addr, addr_end;
>>> + u32 val;
>>> +
>>> + /* Set the ECAM base */
>>> + writel_relaxed(lower_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE);
>>> + writel_relaxed(upper_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE_HI);
>>> +
>>> + /*
>>> + * The only device on root bus is the Root Port. Any access to the PCIe
>>> + * region will go outside the PCIe link. As part of enumeration the PCI
>>> + * sw can try to read to vendor ID & device ID with different device
>>> + * number and function number under root bus. As any access other than
>>> + * root bus, device 0, function 0, should not go out of the link and
>>> + * should return all F's. Since the iATU is configured for the buses
>>> + * which starts after root bus, block the transactions starting from
>>> + * function 1 of the root bus to the end of the root bus (i.e from
>>> + * dbi_base + 4kb to dbi_base + 1MB) from going outside the link.
>>> + */
>>> + addr = pci->dbi_phys_addr + SZ_4K;
>>> + writel_relaxed(lower_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE);
>>> + writel_relaxed(upper_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE_HI);
>>> +
>>> + writel_relaxed(lower_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE);
>>> + writel_relaxed(upper_32_bits(addr), pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE_HI);
>>> +
>>> + addr_end = pci->dbi_phys_addr + SZ_1M - 1;
>>
>> I guess this is an implicit restriction to a single Root Port on the
>> root bus at bb:00.0, right? So when the qcom IP eventually supports
>> multiple Root Ports or even a single Root Port at a different
>> device/function number, this would have to be updated somehow?
>
> The driver already supported ECAM in the existing "firmware_managed"
> path (which looks untouched by this series and doesn't do any of this
> iATU configuration).
>
The firmware_manages doesn't use dwc way of reading/writing to the
config space, except for MSI's that doen't touch dwc core.
> And IIUC, this series adds support for ECAM whenever the DT 'config'
> range is sufficiently aligned. In this new ECAM support, it looks
> like we look for and pay attention to 'bus-range' in this path:
>
> qcom_pcie_probe
> dw_pcie_host_init
> devm_pci_alloc_host_bridge
> devm_of_pci_bridge_init
> pci_parse_request_of_pci_ranges
> devm_of_pci_get_host_bridge_resources
> of_pci_parse_bus_range
> of_property_read_u32_array(node, "bus-range", ...)
> dw_pcie_host_get_resources
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config")
> pp->ecam_enabled = dw_pcie_ecam_enabled(pp, res)
>
> Since qcom_pci_config_ecam() doesn't look at the root bus number at
> all, is this also an implicit restriction that the root bus must be
> bus 0? Does qcom support root buses other than 0?
>
QCOM supports only bus 0.
>>> + writel_relaxed(lower_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT);
>>> + writel_relaxed(upper_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT_HI);
>>> +
>>> + writel_relaxed(lower_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT);
>>> + writel_relaxed(upper_32_bits(addr_end), pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT_HI);
>>> +
>>> + val = readl_relaxed(pcie->parf + PARF_SYS_CTRL);
>>> + val |= PCIE_ECAM_BLOCKER_EN;
>>> + writel_relaxed(val, pcie->parf + PARF_SYS_CTRL);
>>> +}
>>> +
>>> static int qcom_pcie_start_link(struct dw_pcie *pci)
>>> {
>>> struct qcom_pcie *pcie = to_qcom_pcie(pci);
>>> @@ -326,6 +383,9 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
>>> qcom_pcie_common_set_16gt_lane_margining(pci);
>>> }
>>>
>>> + if (pci->pp.ecam_enabled)
>>> + qcom_pci_config_ecam(&pci->pp);
>
> qcom_pcie_start_link() seems like a strange place to do this
> ECAM-related iATU configuration. ECAM is a function of the host
> bridge, not of any particular Root Port or link.
>
There is no API in pci-qcom.c related to the host bridge configuration
currently, as we want to configure before enumeration starts we added
it here, we can move this to qcom_pcie_host_init() if you are ok with
it.
- Krishna Chaitanya.
>>> /* Enable Link Training state machine */
>>> if (pcie->cfg->ops->ltssm_enable)
>>> pcie->cfg->ops->ltssm_enable(pcie);
Powered by blists - more mailing lists