On Thu, Apr 11, 2024 at 11:11 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Thu, Apr 11, 2024 at 09:21:07AM -0500, Rob Herring wrote: > > On Thu, Apr 11, 2024 at 07:39:17AM -0500, Bjorn Helgaas wrote: > > > On Thu, Apr 11, 2024 at 08:13:18AM +0200, Sergio Paracuellos wrote: > > > > On Thu, Apr 11, 2024 at 8:01 AM Krzysztof Kozlowski > > > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > > > On 10/04/2024 23:26, Bjorn Helgaas wrote: > > > > > > On Wed, Apr 10, 2024 at 08:15:19PM +0200, Krzysztof Kozlowski wrote: > > > > > >> MT7621 PCI host bridge has children which apparently are also PCI host > > > > > >> bridges, at least that's what the binding suggest. > > > > > > > > > > > > What does it even mean for a PCI host bridge to have a child that is > > > > > > also a PCI host bridge? > > > > It should say 'root port' instead as the binding description correctly > > says. > > OK, that makes a lot more sense, and we should fix the commit log. > > > > > > I think the question should be towards Mediatek folks. I don't know what > > > > > this hardware is exactly, just looks like pci-pci-bridge. The driver > > > > > calls the children host bridges as "ports". > > > > > > > > You can see the topology here in my first driver submit cover letter > > > > message [0]. > > > > > > > > [0]: https://lore.kernel.org/all/CAMhs-H-BA+KzEwuDPzcmrDPdgJBFA2XdYTBvT4R4MEOUB=WQ1g@xxxxxxxxxxxxxx/t/ > > > > > > Nothing unusual here, this looks like the standard PCIe topology. > > > > > > What *might* be unusual is describing the Root Ports in DT. Since > > > they are standard PCI devices, they shouldn't need DT description > > > unless there's some unusual power/clock/reset control or something > > > that is not discoverable via PCI enumeration. > > > > It's only unusual because typically there's only 1 RP per host bridge > > and properties which really apply to the RP get stuck in the host bridge > > node because we don't have a RP node. An example is perst-gpios. That's > > not a property of the RP either, but the RP is the upstream side of a > > slot and we often don't have a node for the device either. > > Makes sense. > > I'm still confused about one thing, maybe just because I don't really > know how to read these bindings. The binding now looks like this: > > properties: > compatible: > const: mediatek,mt7621-pci > > reg: > items: > - description: host-pci bridge registers > - description: pcie port 0 RC control registers # A > - description: pcie port 1 RC control registers # A > - description: pcie port 2 RC control registers # A > > patternProperties: > '^pcie@[0-2],0$': > type: object > $ref: /schemas/pci/pci-pci-bridge.yaml# > > properties: > reg: # B > maxItems: 1 > > It looks like the "A" items are separate things from the "B" items? > > But I think the relevant code is here: > > mt7621_pcie_probe > mt7621_pcie_parse_dt > pcie->base = devm_platform_ioremap_resource(pdev, 0) # 1 > for_each_available_child_of_node(node, child) > mt7621_pcie_parse_port > port->base = devm_platform_ioremap_resource(pdev, slot + 1) # 2 > > where it looks like both "1" and "2" use the items in the "A" list, > i.e., resources 0, 1, 2, 3, all from the same platform device. Is > there code that uses the "B" item that this patch adds? The A items are in the host address space. The B item is a PCI address. Specifically, for PCI devices, the first entry is config space with just the device and function (devfn). The format is defined in the OpenFirmware PCI bus supplement. Rob
