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

Lorenzo Bianconi
2025-02-21T09:22:54Z

[-- Attachment #1: Type: text/plain, Size: 2357 bytes --]

> [+cc Frank, who asked the same question about DT]
>
> 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?

Regards,
Lorenzo

>
> Bjorn

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]