On Fri, Feb 21, 2025 at 09:30:40AM +0000, Hui Ma (马慧) wrote: > > On Thu, Feb 20, 2025 at 08:54:06PM +0100, Lorenzo Bianconi wrote: > > > On Feb 20, Bjorn Helgaas wrote: > > > > On Sun, Feb 02, 2025 at 08:34:24PM +0100, Lorenzo Bianconi wrote: > > > > > Configure PBus base address and address mask to allow the hw to > > > > > detect if a given address is on PCIE0, PCIE1 or PCIE2. > > > > > > > +#define PCIE_EN7581_PBUS_ADDR(_n) (0x00 + ((_n) << 3)) > > > > > +#define PCIE_EN7581_PBUS_ADDR_MASK(_n) (0x04 + ((_n) << 3)) > > > > > +#define PCIE_EN7581_PBUS_BASE_ADDR(_n) \ > > > > > + ((_n) == 2 ? 0x28000000 : \ > > > > > + (_n) == 1 ? 0x24000000 : 0x20000000) > > > > > > > > Are these addresses something that should be expressed in devicetree? > > > > > > Do you have any example/pointer for it? > > > > > > > It seems unusual to encode addresses directly in a driver. > > > > > > AFAIK they are fixed for EN7581 SoC. > > > > So this is used to detect if a given address is on PCIE0, PCIE1 or > > PCIE2. What does that mean? There are no other mentions of PCIE0 > > etc in the driver, but maybe they match up to "pcie0/1/2" in > > arch/arm64/boot/dts/mediatek/mt7988a.dtsi? > > > > It looks like you use PCIE_EN7581_PBUS_ADDR(slot), where "slot" > > came from of_get_pci_domain_nr(), which suggests that these might > > be three separate Root Ports? > > >I was using pci_domain to detect the specific PCIe controller > >(something similar to what is done here [0]) but I agree with > >Frank, it does not seem completely correct. > > > [0] https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-mediatek.c#L1048 > > > Are we talking about an MMIO address that an endpoint driver uses > > for readw() etc, and this code configures the hardware apertures > > through the host bridge? Seems like that would be related to the > > "ranges" properties in DT. > > >I guess so, but I do not have any documentation about pbus-csr > >(adding Hui in the loop). > > >As pointed out by Frank, do you agree to add these info in the dts? > >Something like: > > >pcie0: pcie@1fc00000 { > > .... > > mediatek,pbus-csr = <&pbus_csr 0x0 0x20000000 0x4 0xfc000000>; > > .... > >}; > > > >pcie1: pcie@1fc20000 { > > .... > > mediatek,pbus-csr = <&pbus_csr 0x8 0x24000000 0xc 0xfc000000>; > > .... > >}; > > >@Hui: can you please provide a better explanation about pbus-csr usage? > > Pbus-csr (base and mask) is used to determine the address > range can be access by PCIe bus. > > 1FBE3400 PCIE0_MEM_BASE 32 PCIE0 base address > 1FBE3404 PCIE0_MEM_MASK 32 PCIE0 base address mask > 1FBE3408 PCIE1_MEM_BASE 32 PCIE1 base address > 1FBE340C PCIE1_MEM_MASK 32 PCIE1 base address mask > 1FBE3410 PCIE2_MEM_BASE 32 PCIE2 base address > 1FBE3414 PCIE2_MEM_MASK 32 PCIE2 base address mask "Can be accessed by PCIe bus" sounds like DMA. Is that what you mean? I doubt it, because if you have multiple host bridges, I assume they would all be able to handle DMA to all of system memory. It would make more sense if this is some sort of description of host bridge apertures, e.g., something like this to allow CPU MMIO accesses to reach the first 2GB of PCI memory space below any of the pcie0, pcie1, pcie2 host bridges: pcie0 0000:00: root bus resource [mem 0x84000000000-0x8407fffffff] (bus address [0x00000000-0x7fffffff]) pcie1 0001:00: root bus resource [mem 0x84100000000-0x8417fffffff] (bus address [0x00000000-0x7fffffff]) pcie2 0002:00: root bus resource [mem 0x84200000000-0x8427fffffff] (bus address [0x00000000-0x7fffffff]) But I think this would be described via 'ranges' properties. And I think it would make sense if the driver had to learn this address map from devicetree and program it into the hardware, so maybe that's what Pbus-csr is for? Total speculation on my part. Bjorn
