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:
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
