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

[Hui Ma (马慧)]

> [+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 at 1fc00000 {
>	....
>	mediatek,pbus-csr = <&pbus_csr 0x0 0x20000000 0x4 0xfc000000>;
>	....
>};
>
>pcie1: pcie at 1fc20000 {
>	....
>	mediatek,pbus-csr = <&pbus_csr 0x8 0x24000000 0xc 0xfc000000>;
>	....
>};

>@Hui: can you please provide a better explanation about pbus-csr usage?

Hi Lorenzo，
	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

>Regards,
>Lorenzo

> 
> Bjorn