Re: 回复: [PATCH v2 2/2] PCI: mediatek-gen3: Configure PBUS_CSR registers for EN7581 SoC

Bjorn Helgaas
2025-02-22T00:09:11Z

On Sat, Feb 22, 2025 at 12:18:52AM +0100, Lorenzo Bianconi wrote:
> [...]
>
> > >
> > > 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.
>
> I agree we should provide these info to the driver via the dts.
>
> Do you agree to pass the register offsets, base address and base mask values
> in the 'mediatek,pbus-csr' phandle array? Something like:
>
> pcie0: pcie@1fc00000 {
> ...
> mediatek,pbus-csr = <&pbus_csr 0x0 0x20000000 0x4 0xfc000000>;
> ...
> }
>
> where:
> - reg offset for base address: 0x0
> - base address value: 0x20000000
> - reg offset for base mask: 0x4
> - base mask value: 0xfc000000
>
> Or do you prefer to just pass register offsets in mediatek,pbus-csr phandle
> array and get base address values reading ranges property? Something like:
>
> pcie0: pcie@1fc00000 {
> ...
> ranges = <0x02000000 0 0x20000000 0x0 0x20000000 0 0x4000000>;
> ...
> mediatek,pbus-csr = <&pbus_csr 0x0 0x4>;
> ...
> }
>
> Considering the latter, even if it is not a real problem for EN7581 since we
> have just a single range, what if we have multiple ranges?

I'm really hesitant about giving DT advice because I don't understand
it well, but I do think it's important to specify host bridge aperture
addresses in 'ranges' because there's lots of generic code that
expects them there.

If you have to program the aperture addresses into the hardware, those
register addresses should be described separately elsewhere. I assume
the aperture size is configurable since you have to write a mask
value, so the driver would have to compute the mask value based on the
aperture size.

Bjorn