Re: [PATCH 3/3] PCI: qcom: Enable ECAM feature based on config size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/5/2024 4:10 AM, Bjorn Helgaas wrote:
On Wed, Dec 04, 2024 at 07:56:54AM +0530, Krishna Chaitanya Chundru wrote:
On 12/4/2024 12:29 AM, Bjorn Helgaas wrote:
On Sun, Nov 17, 2024 at 03:30:20AM +0530, Krishna chaitanya chundru wrote:
Enable the ECAM feature if the config space size is equal to size required
to represent number of buses in the bus range property.

The ELBI registers falls after the DBI space, so use the cfg win returned
from the ecam init to map these regions instead of doing the ioremap again.
ELBI starts at offset 0xf20 from dbi.

On bus 0, we have only the root complex. Any access other than that should
not go out of the link and should return all F's. Since the IATU is
configured for bus 1 onwards, block the transactions for bus 0:0:1 to
0:31:7 (i.e., from dbi_base + 4KB to dbi_base + 1MB) from going outside the
link through ecam blocker through parf registers.

+static bool qcom_pcie_check_ecam_support(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct resource bus_range, *config_res;
+	u64 bus_config_space_count;
+	int ret;
+
+	/* If bus range is not present, keep the bus range as maximum value */
+	ret = of_pci_parse_bus_range(dev->of_node, &bus_range);
+	if (ret) {
+		bus_range.start = 0x0;
+		bus_range.end = 0xff;
+	}

I would have thought the generic OF parsing would already default to
[bus 00-ff]?

if there is no bus-range of_pci_parse_bus_range is not updating it[1],
the bus ranges is being updated to default value in
devm_of_pci_get_host_bridge_resources()[2]

Understood.  But qcom uses dw_pcie_host_init(), which calls
devm_pci_alloc_host_bridge(), which ultimately calls
of_pci_parse_bus_range() and defaults to [bus 00-ff] if there's no
bus-range in DT:

   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(0, 0xff)
               of_pci_parse_bus_range

So the question is why you need to do that again here.

I see that qcom_pcie_probe() calls qcom_pcie_check_ecam_support()
*before* it calls dw_pcie_host_init(), so I guess that's the immediate
answer.

But this is another reason why I think qcom_pcie_check_ecam_support()
is kind of a sub-optimal solution here.

I wonder if we should factor the devm_pci_alloc_host_bridge() call out
of dw_pcie_host_init() so drivers can take advantage of the DT parsing
it does.  It looks like mobiveil does it that way:
It makes sense to use this way in the next patch in the qcom driver will
call devm_pci_alloc_host_bridge() before calling dw_pcie_host_init() and
in dw_pcie_host_init() if the bridge is allocated dwc driver will skip
allocating the bridge so that other drivers will not be affected.

- Krishna Chaitanya.


   ls_g4_pcie_probe
     devm_pci_alloc_host_bridge
     mobiveil_pcie_host_probe

   mobiveil_pcie_probe
     devm_pci_alloc_host_bridge
     mobiveil_pcie_host_probe

[1]https://elixir.bootlin.com/linux/v6.12.1/source/drivers/pci/of.c#L193
[2]https://elixir.bootlin.com/linux/v6.12.1/source/drivers/pci/of.c#L347





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux