[PATCH v2 0/2] PCI: Configure max payload size on pci_host_probe

[PATCH v2 0/2] PCI: Configure max payload size on pci_host_probe

[Hans Zhang]

1. PCI: Configure root port MPS to hardware maximum during host probing.
2. PCI: Remove redundant MPS configuration.

---
Changes for v2:
- According to the Maintainer's suggestion, limit the setting of MPS
  changes to platforms with controller drivers.
- Delete the MPS code set by the SOC manufacturer.
---

Hans Zhang (2):
  PCI: Configure root port MPS to hardware maximum during host probing
  PCI: Remove redundant MPS configuration

 drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
 drivers/pci/controller/pci-aardvark.c  |  2 --
 drivers/pci/probe.c                    | 12 ++++++++++++
 3 files changed, 12 insertions(+), 19 deletions(-)


base-commit: 9d7a0577c9db35c4cc52db90bc415ea248446472
-- 
2.25.1

[PATCH v2 1/2] PCI: Configure root port MPS to hardware maximum during host probing

[Hans Zhang]

Current PCIe initialization logic may leave root ports operating with
non-optimal Maximum Payload Size (MPS) settings. While downstream device
configuration is handled during bus enumeration, root port MPS values
inherited from firmware or hardware defaults might not utilize the full
capabilities supported by the controller hardware. This can result in
suboptimal data transfer efficiency across the PCIe hierarchy.

During host controller probing phase, when PCIe bus tuning is enabled,
the implementation now configures root port MPS settings to their
hardware-supported maximum values. By iterating through bridge devices
under the root bus and identifying PCIe root ports, each port's MPS is set
to 128 << pcie_mpss to match the device's maximum supported payload size.
The Max Read Request Size (MRRS) is subsequently adjusted through existing
companion logic to maintain compatibility with PCIe specifications.

Explicit initialization at host probing stage ensures consistent PCIe
topology configuration before downstream devices perform their own MPS
negotiations. This proactive approach addresses platform-specific
requirements where controller drivers depend on properly initialized root
port settings, while maintaining backward compatibility through
PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
utilized without altering existing device negotiation behaviors.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/probe.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 364fa2a514f8..3973c593fdcf 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3206,6 +3206,7 @@ EXPORT_SYMBOL_GPL(pci_create_root_bus);
 int pci_host_probe(struct pci_host_bridge *bridge)
 {
 	struct pci_bus *bus, *child;
+	struct pci_dev *dev;
 	int ret;
 
 	pci_lock_rescan_remove();
@@ -3228,6 +3229,17 @@ int pci_host_probe(struct pci_host_bridge *bridge)
 	 */
 	pci_assign_unassigned_root_bus_resources(bus);
 
+	if (pcie_bus_config != PCIE_BUS_TUNE_OFF) {
+		/* Configure root ports MPS to be MPSS by default */
+		for_each_pci_bridge(dev, bus) {
+			if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
+				continue;
+
+			pcie_write_mps(dev, 128 << dev->pcie_mpss);
+			pcie_write_mrrs(dev);
+		}
+	}
+
 	list_for_each_entry(child, &bus->children, node)
 		pcie_bus_configure_settings(child);
 
-- 
2.25.1

[PATCH v2 2/2] PCI: Remove redundant MPS configuration

[Hans Zhang]

With the PCI core now centrally configuring root port MPS to
hardware-supported maximums (via 128 << pcie_mpss) during host probing,
platform-specific MPS adjustments are redundant. This patch removes the
custom the configuration of the max payload logic to align with the
standardized initialization flow.

By eliminating redundant code, this change prevents conflicts with global
PCIe hierarchy tuning policies and reduces maintenance overhead. The Meson
driver now fully relies on the core PCI framework for MPS configuration,
ensuring consistency across the PCIe topology while preserving
hardware-specific MRRS handling.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
 drivers/pci/controller/pci-aardvark.c  |  2 --
 2 files changed, 19 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index db9482a113e9..126f38ed453d 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -261,22 +261,6 @@ static int meson_size_to_payload(struct meson_pcie *mp, int size)
 	return fls(size) - 8;
 }
 
-static void meson_set_max_payload(struct meson_pcie *mp, int size)
-{
-	struct dw_pcie *pci = &mp->pci;
-	u32 val;
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	int max_payload_size = meson_size_to_payload(mp, size);
-
-	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
-	val &= ~PCI_EXP_DEVCTL_PAYLOAD;
-	dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
-
-	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
-	val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
-	dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
-}
-
 static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
 {
 	struct dw_pcie *pci = &mp->pci;
@@ -381,7 +365,6 @@ static int meson_pcie_host_init(struct dw_pcie_rp *pp)
 
 	pp->bridge->ops = &meson_pci_ops;
 
-	meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
 	meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);
 
 	return 0;
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index a29796cce420..d8852892994a 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -549,9 +549,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 	reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
 	reg &= ~PCI_EXP_DEVCTL_RELAX_EN;
 	reg &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
-	reg &= ~PCI_EXP_DEVCTL_PAYLOAD;
 	reg &= ~PCI_EXP_DEVCTL_READRQ;
-	reg |= PCI_EXP_DEVCTL_PAYLOAD_512B;
 	reg |= PCI_EXP_DEVCTL_READRQ_512B;
 	advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
 
-- 
2.25.1

Re: [PATCH v2 2/2] PCI: Remove redundant MPS configuration

[Niklas Cassel]

On Fri, Apr 25, 2025 at 05:57:08PM +0800, Hans Zhang wrote:
> With the PCI core now centrally configuring root port MPS to
> hardware-supported maximums (via 128 << pcie_mpss) during host probing,
> platform-specific MPS adjustments are redundant. This patch removes the
> custom the configuration of the max payload logic to align with the
> standardized initialization flow.
> 
> By eliminating redundant code, this change prevents conflicts with global
> PCIe hierarchy tuning policies and reduces maintenance overhead. The Meson
> driver now fully relies on the core PCI framework for MPS configuration,
> ensuring consistency across the PCIe topology while preserving
> hardware-specific MRRS handling.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
>  drivers/pci/controller/pci-aardvark.c  |  2 --

Since you are touching two drivers (and the changes are not exactly identical),
I suggest that you do one patch per driver.


Kind regards,
Niklas

Re: [PATCH v2 1/2] PCI: Configure root port MPS to hardware maximum during host probing

[Niklas Cassel]

On Fri, Apr 25, 2025 at 05:57:07PM +0800, Hans Zhang wrote:
> Current PCIe initialization logic may leave root ports operating with
> non-optimal Maximum Payload Size (MPS) settings. While downstream device
> configuration is handled during bus enumeration, root port MPS values
> inherited from firmware or hardware defaults might not utilize the full
> capabilities supported by the controller hardware. This can result in
> suboptimal data transfer efficiency across the PCIe hierarchy.
> 
> During host controller probing phase, when PCIe bus tuning is enabled,
> the implementation now configures root port MPS settings to their
> hardware-supported maximum values. By iterating through bridge devices
> under the root bus and identifying PCIe root ports, each port's MPS is set
> to 128 << pcie_mpss to match the device's maximum supported payload size.
> The Max Read Request Size (MRRS) is subsequently adjusted through existing
> companion logic to maintain compatibility with PCIe specifications.
> 
> Explicit initialization at host probing stage ensures consistent PCIe
> topology configuration before downstream devices perform their own MPS
> negotiations. This proactive approach addresses platform-specific
> requirements where controller drivers depend on properly initialized root
> port settings, while maintaining backward compatibility through
> PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
> utilized without altering existing device negotiation behaviors.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>

Perhaps Mani deserves a Suggested-by tag?


> ---
>  drivers/pci/probe.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 364fa2a514f8..3973c593fdcf 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -3206,6 +3206,7 @@ EXPORT_SYMBOL_GPL(pci_create_root_bus);
>  int pci_host_probe(struct pci_host_bridge *bridge)
>  {
>  	struct pci_bus *bus, *child;
> +	struct pci_dev *dev;
>  	int ret;
>  
>  	pci_lock_rescan_remove();
> @@ -3228,6 +3229,17 @@ int pci_host_probe(struct pci_host_bridge *bridge)
>  	 */
>  	pci_assign_unassigned_root_bus_resources(bus);
>  
> +	if (pcie_bus_config != PCIE_BUS_TUNE_OFF) {
> +		/* Configure root ports MPS to be MPSS by default */
> +		for_each_pci_bridge(dev, bus) {
> +			if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
> +				continue;
> +
> +			pcie_write_mps(dev, 128 << dev->pcie_mpss);
> +			pcie_write_mrrs(dev);

The comment says configure MPS, but the code also calls pcie_write_mrrs().

Should we update the comment or should we remove the call to pcie_write_mrrs()?

Note that even when calling pcie_write_mrrs(), it will not update MRRS for the
common case (pcie_bus_config == PCIE_BUS_DEFAULT).


Kind regards,
Niklas

Re: [PATCH v2 2/2] PCI: Remove redundant MPS configuration

[Hans Zhang]

On 2025/4/25 18:17, Niklas Cassel wrote:
> On Fri, Apr 25, 2025 at 05:57:08PM +0800, Hans Zhang wrote:
>> With the PCI core now centrally configuring root port MPS to
>> hardware-supported maximums (via 128 << pcie_mpss) during host probing,
>> platform-specific MPS adjustments are redundant. This patch removes the
>> custom the configuration of the max payload logic to align with the
>> standardized initialization flow.
>>
>> By eliminating redundant code, this change prevents conflicts with global
>> PCIe hierarchy tuning policies and reduces maintenance overhead. The Meson
>> driver now fully relies on the core PCI framework for MPS configuration,
>> ensuring consistency across the PCIe topology while preserving
>> hardware-specific MRRS handling.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
>>   drivers/pci/controller/pci-aardvark.c  |  2 --
> 
> Since you are touching two drivers (and the changes are not exactly identical),
> I suggest that you do one patch per driver.

Dear Niklas,

Thank you very much for your reply. In the next version, I will split 
two patches.

Best regards,
Hans

Re: [PATCH v2 1/2] PCI: Configure root port MPS to hardware maximum during host probing

[Hans Zhang]

On 2025/4/25 18:23, Niklas Cassel wrote:
> On Fri, Apr 25, 2025 at 05:57:07PM +0800, Hans Zhang wrote:
>> Current PCIe initialization logic may leave root ports operating with
>> non-optimal Maximum Payload Size (MPS) settings. While downstream device
>> configuration is handled during bus enumeration, root port MPS values
>> inherited from firmware or hardware defaults might not utilize the full
>> capabilities supported by the controller hardware. This can result in
>> suboptimal data transfer efficiency across the PCIe hierarchy.
>>
>> During host controller probing phase, when PCIe bus tuning is enabled,
>> the implementation now configures root port MPS settings to their
>> hardware-supported maximum values. By iterating through bridge devices
>> under the root bus and identifying PCIe root ports, each port's MPS is set
>> to 128 << pcie_mpss to match the device's maximum supported payload size.
>> The Max Read Request Size (MRRS) is subsequently adjusted through existing
>> companion logic to maintain compatibility with PCIe specifications.
>>
>> Explicit initialization at host probing stage ensures consistent PCIe
>> topology configuration before downstream devices perform their own MPS
>> negotiations. This proactive approach addresses platform-specific
>> requirements where controller drivers depend on properly initialized root
>> port settings, while maintaining backward compatibility through
>> PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
>> utilized without altering existing device negotiation behaviors.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
> 
> Perhaps Mani deserves a Suggested-by tag?
> 

Dear Niklas,

Thank you very much for your reply. Ok. Sorry, I missed it. I 'm going 
to add Suggested-by tag.

> 
>> ---
>>   drivers/pci/probe.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 364fa2a514f8..3973c593fdcf 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -3206,6 +3206,7 @@ EXPORT_SYMBOL_GPL(pci_create_root_bus);
>>   int pci_host_probe(struct pci_host_bridge *bridge)
>>   {
>>   	struct pci_bus *bus, *child;
>> +	struct pci_dev *dev;
>>   	int ret;
>>   
>>   	pci_lock_rescan_remove();
>> @@ -3228,6 +3229,17 @@ int pci_host_probe(struct pci_host_bridge *bridge)
>>   	 */
>>   	pci_assign_unassigned_root_bus_resources(bus);
>>   
>> +	if (pcie_bus_config != PCIE_BUS_TUNE_OFF) {
>> +		/* Configure root ports MPS to be MPSS by default */
>> +		for_each_pci_bridge(dev, bus) {
>> +			if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
>> +				continue;
>> +
>> +			pcie_write_mps(dev, 128 << dev->pcie_mpss);
>> +			pcie_write_mrrs(dev);
> 
> The comment says configure MPS, but the code also calls pcie_write_mrrs().
> 
> Should we update the comment or should we remove the call to pcie_write_mrrs()?
> 

I have tested and found that the result is the same whether 
pcie_write_mrrs() is called or not.

> Note that even when calling pcie_write_mrrs(), it will not update MRRS for the
> common case (pcie_bus_config == PCIE_BUS_DEFAULT).

But I discovered a problem:

0001:90:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
          ......
          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0
                          ExtTag- RBE+
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                          MaxPayload 512 bytes, MaxReadReq 1024 bytes



			Should the DevCtl MaxPayload be 256B?

But I tested that the file reading and writing were normal. Is the 
display of 512B here what we expected?

Root Port 0003:30:00.0 has the same problem. May I ask what your opinion is?


		......
0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller PM9A1/PM9A3/980PRO (prog-if 02 [NVM Express])
          ......
          Capabilities: [70] Express (v2) Endpoint, MSI 00
                  DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
                          ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0W
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
FLReset-
                          MaxPayload 256 bytes, MaxReadReq 512 bytes
		......





Several PCIe ports that I enabled.

root@cix-localhost:~# cat /proc/version
Linux version 6.15.0-rc2-00015-gced1536aaf04-dirty (hans@hans) ......
root@cix-localhost:~# lspci
0000:c0:00.0 PCI bridge: Device 1f6c:0001
0000:c1:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller S4LV008[Pascal]
0001:90:00.0 PCI bridge: Device 1f6c:0001
0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller PM9A1/PM9A3/980PRO
0003:30:00.0 PCI bridge: Device 1f6c:0001
0003:31:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller (rev 05)root@cix-localhost:~# lspci -vvv
0000:c0:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
          ......
          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0
                          ExtTag+ RBE+
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
                          MaxPayload 512 bytes, MaxReadReq 1024 bytes
		......
0000:c1:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller S4LV008[Pascal] (prog-if 02 [NVM Express])
          ......
          Capabilities: [70] Express (v2) Endpoint, MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
                          ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0W
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
FLReset-
                          MaxPayload 512 bytes, MaxReadReq 512 bytes
		......
0001:90:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
          ......
          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0
                          ExtTag- RBE+
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                          MaxPayload 512 bytes, MaxReadReq 1024 bytes
		......
0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller PM9A1/PM9A3/980PRO (prog-if 02 [NVM Express])
          ......
          Capabilities: [70] Express (v2) Endpoint, MSI 00
                  DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
                          ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0W
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
FLReset-
                          MaxPayload 256 bytes, MaxReadReq 512 bytes
		......
0003:30:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
          ......
          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                  DevCap: MaxPayload 512 bytes, PhantFunc 0
                          ExtTag- RBE+
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                          MaxPayload 512 bytes, MaxReadReq 1024 bytes
		......
0003:31:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller (rev 05)
          ......
          Capabilities: [70] Express (v2) Endpoint, MSI 01
                  DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
<512ns, L1 <64us
                          ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
SlotPowerLimit 0W
                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
                          MaxPayload 256 bytes, MaxReadReq 4096 bytes
          ......

Best regards,
Hans

Re: [PATCH v2 2/2] PCI: Remove redundant MPS configuration

[neil.armstrong]

On 25/04/2025 11:57, Hans Zhang wrote:
> With the PCI core now centrally configuring root port MPS to
> hardware-supported maximums (via 128 << pcie_mpss) during host probing,
> platform-specific MPS adjustments are redundant. This patch removes the
> custom the configuration of the max payload logic to align with the
> standardized initialization flow.
> 
> By eliminating redundant code, this change prevents conflicts with global
> PCIe hierarchy tuning policies and reduces maintenance overhead. The Meson
> driver now fully relies on the core PCI framework for MPS configuration,
> ensuring consistency across the PCIe topology while preserving
> hardware-specific MRRS handling.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>   drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
>   drivers/pci/controller/pci-aardvark.c  |  2 --
>   2 files changed, 19 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index db9482a113e9..126f38ed453d 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -261,22 +261,6 @@ static int meson_size_to_payload(struct meson_pcie *mp, int size)
>   	return fls(size) - 8;
>   }
>   
> -static void meson_set_max_payload(struct meson_pcie *mp, int size)
> -{
> -	struct dw_pcie *pci = &mp->pci;
> -	u32 val;
> -	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> -	int max_payload_size = meson_size_to_payload(mp, size);
> -
> -	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
> -	val &= ~PCI_EXP_DEVCTL_PAYLOAD;
> -	dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
> -
> -	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
> -	val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
> -	dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
> -}
> -
>   static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
>   {
>   	struct dw_pcie *pci = &mp->pci;
> @@ -381,7 +365,6 @@ static int meson_pcie_host_init(struct dw_pcie_rp *pp)
>   
>   	pp->bridge->ops = &meson_pci_ops;
>   
> -	meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
>   	meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);

Seems you can also remove meson_set_max_rd_req_size() since it's
done by pcie_write_mrrs()

Neil

>   
>   	return 0;
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index a29796cce420..d8852892994a 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -549,9 +549,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
>   	reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
>   	reg &= ~PCI_EXP_DEVCTL_RELAX_EN;
>   	reg &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
> -	reg &= ~PCI_EXP_DEVCTL_PAYLOAD;
>   	reg &= ~PCI_EXP_DEVCTL_READRQ;
> -	reg |= PCI_EXP_DEVCTL_PAYLOAD_512B;
>   	reg |= PCI_EXP_DEVCTL_READRQ_512B;
>   	advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
>   

Re: [PATCH v2 1/2] PCI: Configure root port MPS to hardware maximum during host probing

[Niklas Cassel]

Hello Hans,

On Fri, Apr 25, 2025 at 06:56:53PM +0800, Hans Zhang wrote:
> 
> But I discovered a problem:
> 
> 0001:90:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
>          ......
>          Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
>                  DevCap: MaxPayload 512 bytes, PhantFunc 0
>                          ExtTag- RBE+
>                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
>                          RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>                          MaxPayload 512 bytes, MaxReadReq 1024 bytes
> 
> 
> 
> 			Should the DevCtl MaxPayload be 256B?
> 
> But I tested that the file reading and writing were normal. Is the display
> of 512B here what we expected?
> 
> Root Port 0003:30:00.0 has the same problem. May I ask what your opinion is?
> 
> 
> 		......
> 0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
> NVMe SSD Controller PM9A1/PM9A3/980PRO (prog-if 02 [NVM Express])
>          ......
>          Capabilities: [70] Express (v2) Endpoint, MSI 00
>                  DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
> unlimited, L1 unlimited
>                          ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
> SlotPowerLimit 0W
>                  DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
>                          RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
> FLReset-
>                          MaxPayload 256 bytes, MaxReadReq 512 bytes
> 		......

Here we see that the bridge has a higher DevCtl.MPS than the DevCap.MPS of
the endpoint.

Let me quote Bjorn from the previous mail thread:

"""
  - I don't think it's safe to set MPS higher in all cases.  If we set
    the Root Port MPS=256, and an Endpoint only supports MPS=128, the
    Endpoint may do a 256-byte DMA read (assuming its MRRS>=256).  In
    that case the RP may respond with a 256-byte payload the Endpoint
    can't handle.
"""



I think the problem with this patch is that pcie_write_mps() call in
pci_host_probe() is done after the pci_scan_root_bus_bridge() call in
pci_host_probe().

So pci_configure_mps() (called by pci_configure_device()),
which does the limiting of the bus to what the endpoint supports,
is actually called before the pcie_write_mps() call added by this patch
(which increases DevCtl.MPS for the bridge).


So I think the code added in this patch needs to be executed before
pci_configure_device() is done for the EP.

It appears that pci_configure_device() is called for each device
during scan, first for the bridges and then for the EPs.

So I think something like this should work (totally untested):

--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -45,6 +45,8 @@ struct pci_domain_busn_res {
        int domain_nr;
 };
 
+static void pcie_write_mps(struct pci_dev *dev, int mps);
+
 static struct resource *get_pci_domain_busn_res(int domain_nr)
 {
        struct pci_domain_busn_res *r;
@@ -2178,6 +2180,11 @@ static void pci_configure_mps(struct pci_dev *dev)
                return;
        }
 
+       if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
+           pcie_bus_config != PCIE_BUS_TUNE_OFF) {
+               pcie_write_mps(dev, 128 << dev->pcie_mpss);
+       }
+
        if (!bridge || !pci_is_pcie(bridge))
                return;



But we would probably need to move some code to avoid the
forward declaration.


Kind regards,
Niklas

Re: [PATCH v2 1/2] PCI: Configure root port MPS to hardware maximum during host probing

[Hans Zhang]

On 2025/4/25 21:47, Niklas Cassel wrote:
> Hello Hans,
> 
> On Fri, Apr 25, 2025 at 06:56:53PM +0800, Hans Zhang wrote:
>>
>> But I discovered a problem:
>>
>> 0001:90:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
>>           ......
>>           Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
>>                   DevCap: MaxPayload 512 bytes, PhantFunc 0
>>                           ExtTag- RBE+
>>                   DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
>>                           RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>                           MaxPayload 512 bytes, MaxReadReq 1024 bytes
>>
>>
>>
>> 			Should the DevCtl MaxPayload be 256B?
>>
>> But I tested that the file reading and writing were normal. Is the display
>> of 512B here what we expected?
>>
>> Root Port 0003:30:00.0 has the same problem. May I ask what your opinion is?
>>
>>
>> 		......
>> 0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
>> NVMe SSD Controller PM9A1/PM9A3/980PRO (prog-if 02 [NVM Express])
>>           ......
>>           Capabilities: [70] Express (v2) Endpoint, MSI 00
>>                   DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
>> unlimited, L1 unlimited
>>                           ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
>> SlotPowerLimit 0W
>>                   DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
>>                           RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
>> FLReset-
>>                           MaxPayload 256 bytes, MaxReadReq 512 bytes
>> 		......
> 
> Here we see that the bridge has a higher DevCtl.MPS than the DevCap.MPS of
> the endpoint.
> 
> Let me quote Bjorn from the previous mail thread:
> 
> """
>    - I don't think it's safe to set MPS higher in all cases.  If we set
>      the Root Port MPS=256, and an Endpoint only supports MPS=128, the
>      Endpoint may do a 256-byte DMA read (assuming its MRRS>=256).  In
>      that case the RP may respond with a 256-byte payload the Endpoint
>      can't handle.
> """
> 
> 
> 
> I think the problem with this patch is that pcie_write_mps() call in
> pci_host_probe() is done after the pci_scan_root_bus_bridge() call in
> pci_host_probe().
> 
> So pci_configure_mps() (called by pci_configure_device()),
> which does the limiting of the bus to what the endpoint supports,
> is actually called before the pcie_write_mps() call added by this patch
> (which increases DevCtl.MPS for the bridge).
> 
> 
> So I think the code added in this patch needs to be executed before
> pci_configure_device() is done for the EP.
> 
> It appears that pci_configure_device() is called for each device
> during scan, first for the bridges and then for the EPs.
> 
> So I think something like this should work (totally untested):
> 
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -45,6 +45,8 @@ struct pci_domain_busn_res {
>          int domain_nr;
>   };
>   
> +static void pcie_write_mps(struct pci_dev *dev, int mps);
> +
>   static struct resource *get_pci_domain_busn_res(int domain_nr)
>   {
>          struct pci_domain_busn_res *r;
> @@ -2178,6 +2180,11 @@ static void pci_configure_mps(struct pci_dev *dev)
>                  return;
>          }
>   
> +       if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
> +           pcie_bus_config != PCIE_BUS_TUNE_OFF) {
> +               pcie_write_mps(dev, 128 << dev->pcie_mpss);
> +       }
> +
>          if (!bridge || !pci_is_pcie(bridge))
>                  return;
> 
> 
> 
> But we would probably need to move some code to avoid the
> forward declaration.
> 

Dear Niklas,

Thank you very much for your reply and suggestions. The patch you 
provided has been tested by me and is normal.



Bjorn and Mani, thoughts?


Please see the following log:

lspci -vvv
0000:c0:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
         ......
         Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                 DevCap: MaxPayload 512 bytes, PhantFunc 0
                         ExtTag+ RBE+
                 DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                         RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
                         MaxPayload 512 bytes, MaxReadReq 1024 bytes
         ......

0000:c1:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd 
NVMe SSD Controller S4LV008[Pascal] (prog-if 02 [NVM Express])
         ......
         Capabilities: [70] Express (v2) Endpoint, MSI 00
                 DevCap: MaxPayload 512 bytes, PhantFunc 0, Latency L0s 
unlimited, L1 unlimited
                         ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+ 
SlotPowerLimit 0W
                 DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                         RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+ 
FLReset-
                         MaxPayload 512 bytes, MaxReadReq 512 bytes
         ......

0001:90:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
         ......
         Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                 DevCap: MaxPayload 512 bytes, PhantFunc 0
                         ExtTag- RBE+
                 DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                         MaxPayload 256 bytes, MaxReadReq 1024 bytes
         ......

0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd 
NVMe SSD Controller PM9A1/PM9A3/980PRO (prog-if 02 [NVM Express])
         ......
         Capabilities: [70] Express (v2) Endpoint, MSI 00
                 DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s 
unlimited, L1 unlimited
                         ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+ 
SlotPowerLimit 0W
                 DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                         RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+ 
FLReset-
                         MaxPayload 256 bytes, MaxReadReq 512 bytes
         ......

0003:30:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
         ......
         Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                 DevCap: MaxPayload 512 bytes, PhantFunc 0
                         ExtTag- RBE+
                 DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                         MaxPayload 256 bytes, MaxReadReq 1024 bytes
         ......

0003:31:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. 
RTL8125 2.5GbE Controller (rev 05)
         ......
         Capabilities: [70] Express (v2) Endpoint, MSI 01
                 DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s 
<512ns, L1 <64us
                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- 
SlotPowerLimit 0W
                 DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
                         MaxPayload 256 bytes, MaxReadReq 4096 bytes
         ......

0004:00:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
         ......
         Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
                 DevCap: MaxPayload 512 bytes, PhantFunc 0
                         ExtTag- RBE+
                 DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                         MaxPayload 256 bytes, MaxReadReq 1024 bytes
         ......

0004:01:00.0 Network controller: Realtek Semiconductor Co., Ltd. 
RTL8852BE PCIe 802.11ax Wireless Network Controller
         ......
         Capabilities: [70] Express (v2) Endpoint, MSI 00
                 DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s 
<4us, L1 <64us
                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset+ 
SlotPowerLimit 0W
                 DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
                         RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop- 
FLReset-
                         MaxPayload 256 bytes, MaxReadReq 512 bytes
         ......


Best regards,
Hans

Re: [PATCH v2 2/2] PCI: Remove redundant MPS configuration

[Hans Zhang]

On 2025/4/25 19:59, neil.armstrong@linaro.org wrote:
> On 25/04/2025 11:57, Hans Zhang wrote:
>> With the PCI core now centrally configuring root port MPS to
>> hardware-supported maximums (via 128 << pcie_mpss) during host probing,
>> platform-specific MPS adjustments are redundant. This patch removes the
>> custom the configuration of the max payload logic to align with the
>> standardized initialization flow.
>>
>> By eliminating redundant code, this change prevents conflicts with global
>> PCIe hierarchy tuning policies and reduces maintenance overhead. The 
>> Meson
>> driver now fully relies on the core PCI framework for MPS configuration,
>> ensuring consistency across the PCIe topology while preserving
>> hardware-specific MRRS handling.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
>>   drivers/pci/controller/pci-aardvark.c  |  2 --
>>   2 files changed, 19 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pci-meson.c 
>> b/drivers/pci/controller/dwc/pci-meson.c
>> index db9482a113e9..126f38ed453d 100644
>> --- a/drivers/pci/controller/dwc/pci-meson.c
>> +++ b/drivers/pci/controller/dwc/pci-meson.c
>> @@ -261,22 +261,6 @@ static int meson_size_to_payload(struct 
>> meson_pcie *mp, int size)
>>       return fls(size) - 8;
>>   }
>> -static void meson_set_max_payload(struct meson_pcie *mp, int size)
>> -{
>> -    struct dw_pcie *pci = &mp->pci;
>> -    u32 val;
>> -    u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> -    int max_payload_size = meson_size_to_payload(mp, size);
>> -
>> -    val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
>> -    val &= ~PCI_EXP_DEVCTL_PAYLOAD;
>> -    dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
>> -
>> -    val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
>> -    val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
>> -    dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
>> -}
>> -
>>   static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
>>   {
>>       struct dw_pcie *pci = &mp->pci;
>> @@ -381,7 +365,6 @@ static int meson_pcie_host_init(struct dw_pcie_rp 
>> *pp)
>>       pp->bridge->ops = &meson_pci_ops;
>> -    meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
>>       meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);
> 
> Seems you can also remove meson_set_max_rd_req_size() since it's
> done by pcie_write_mrrs()


Dear neil,

Thank you very much for your reply and reminder.

I want to wait for the result of the first patch discussion, and then 
see if we need to remove meson_set_max_rd_req_size().

Best regards,
Hans

Re: [PATCH v2 2/2] PCI: Remove redundant MPS configuration

[Pali Rohár]

On Friday 25 April 2025 17:57:08 Hans Zhang wrote:
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index a29796cce420..d8852892994a 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -549,9 +549,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
>  	reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
>  	reg &= ~PCI_EXP_DEVCTL_RELAX_EN;
>  	reg &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
> -	reg &= ~PCI_EXP_DEVCTL_PAYLOAD;
>  	reg &= ~PCI_EXP_DEVCTL_READRQ;
> -	reg |= PCI_EXP_DEVCTL_PAYLOAD_512B;
>  	reg |= PCI_EXP_DEVCTL_READRQ_512B;
>  	advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
>  
> -- 
> 2.25.1
> 

Please do not remove this code. It is required part of the
initialization of the aardvark PCI controller at the specific phase,
as defined in the Armada 3700 Functional Specification.

Re: [PATCH v2 2/2] PCI: Remove redundant MPS configuration

[Hans Zhang]

On 2025/4/26 02:13, Pali Rohár wrote:
> On Friday 25 April 2025 17:57:08 Hans Zhang wrote:
>> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
>> index a29796cce420..d8852892994a 100644
>> --- a/drivers/pci/controller/pci-aardvark.c
>> +++ b/drivers/pci/controller/pci-aardvark.c
>> @@ -549,9 +549,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
>>   	reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
>>   	reg &= ~PCI_EXP_DEVCTL_RELAX_EN;
>>   	reg &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
>> -	reg &= ~PCI_EXP_DEVCTL_PAYLOAD;
>>   	reg &= ~PCI_EXP_DEVCTL_READRQ;
>> -	reg |= PCI_EXP_DEVCTL_PAYLOAD_512B;
>>   	reg |= PCI_EXP_DEVCTL_READRQ_512B;
>>   	advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
>>   
>> -- 
>> 2.25.1
>>
> 
> Please do not remove this code. It is required part of the
> initialization of the aardvark PCI controller at the specific phase,
> as defined in the Armada 3700 Functional Specification.

Hi Pali,

This series of patches is discussing the initialization of DevCtl.MPS by 
the Root Port. Please look at the first patch I submitted. If there is a 
reasonable method in the end, DevCtl.MPS will also be configured 
successfully. The PCIe maintainer will give the review opinions. Please 
rest assured that it will not affect the functions of the aardvark PCI 
controller.

Rockchip's RK3588 also has the same problem.


https://patchwork.kernel.org/project/linux-pci/patch/20250416151926.140202-1-18255117159@163.com/

Best regards,
Hans

Re: [PATCH v2 2/2] PCI: Remove redundant MPS configuration

[Pali Rohár]

On Saturday 26 April 2025 23:02:08 Hans Zhang wrote:
> On 2025/4/26 02:13, Pali Rohár wrote:
> > On Friday 25 April 2025 17:57:08 Hans Zhang wrote:
> > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > index a29796cce420..d8852892994a 100644
> > > --- a/drivers/pci/controller/pci-aardvark.c
> > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > @@ -549,9 +549,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
> > >   	reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
> > >   	reg &= ~PCI_EXP_DEVCTL_RELAX_EN;
> > >   	reg &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
> > > -	reg &= ~PCI_EXP_DEVCTL_PAYLOAD;
> > >   	reg &= ~PCI_EXP_DEVCTL_READRQ;
> > > -	reg |= PCI_EXP_DEVCTL_PAYLOAD_512B;
> > >   	reg |= PCI_EXP_DEVCTL_READRQ_512B;
> > >   	advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
> > > -- 
> > > 2.25.1
> > > 
> > 
> > Please do not remove this code. It is required part of the
> > initialization of the aardvark PCI controller at the specific phase,
> > as defined in the Armada 3700 Functional Specification.
> 
> Hi Pali,
> 
> This series of patches is discussing the initialization of DevCtl.MPS by the
> Root Port. Please look at the first patch I submitted. If there is a
> reasonable method in the end, DevCtl.MPS will also be configured
> successfully.

This does not matter what would be configured in DevCtl.MPS at the end.

> The PCIe maintainer will give the review opinions. Please rest
> assured that it will not affect the functions of the aardvark PCI
> controller.

This patch is modifying initialization of the aardvark PCIe controller
and is removing the mandatory step of the controller configuration as
required and defined in the Armada 3700 Functional Specification.
It says exactly in which order and which values to which registers has
to be written.

> Rockchip's RK3588 also has the same problem.
> 
> 
> https://patchwork.kernel.org/project/linux-pci/patch/20250416151926.140202-1-18255117159@163.com/
> 
> Best regards,
> Hans
> 

Re: [PATCH v2 2/2] PCI: Remove redundant MPS configuration

[Hans Zhang]

On 2025/4/26 23:06, Pali Rohár wrote:
> On Saturday 26 April 2025 23:02:08 Hans Zhang wrote:
>> On 2025/4/26 02:13, Pali Rohár wrote:
>>> On Friday 25 April 2025 17:57:08 Hans Zhang wrote:
>>>> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
>>>> index a29796cce420..d8852892994a 100644
>>>> --- a/drivers/pci/controller/pci-aardvark.c
>>>> +++ b/drivers/pci/controller/pci-aardvark.c
>>>> @@ -549,9 +549,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
>>>>    	reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
>>>>    	reg &= ~PCI_EXP_DEVCTL_RELAX_EN;
>>>>    	reg &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
>>>> -	reg &= ~PCI_EXP_DEVCTL_PAYLOAD;
>>>>    	reg &= ~PCI_EXP_DEVCTL_READRQ;
>>>> -	reg |= PCI_EXP_DEVCTL_PAYLOAD_512B;
>>>>    	reg |= PCI_EXP_DEVCTL_READRQ_512B;
>>>>    	advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_DEVCTL);
>>>> -- 
>>>> 2.25.1
>>>>
>>>
>>> Please do not remove this code. It is required part of the
>>> initialization of the aardvark PCI controller at the specific phase,
>>> as defined in the Armada 3700 Functional Specification.
>>
>> Hi Pali,
>>
>> This series of patches is discussing the initialization of DevCtl.MPS by the
>> Root Port. Please look at the first patch I submitted. If there is a
>> reasonable method in the end, DevCtl.MPS will also be configured
>> successfully.
> 
> This does not matter what would be configured in DevCtl.MPS at the end.
> 
>> The PCIe maintainer will give the review opinions. Please rest
>> assured that it will not affect the functions of the aardvark PCI
>> controller.
> 
> This patch is modifying initialization of the aardvark PCIe controller
> and is removing the mandatory step of the controller configuration as
> required and defined in the Armada 3700 Functional Specification.
> It says exactly in which order and which values to which registers has
> to be written.

Hi Pali,

Is the maximum MPS supported by Armada 3700 512 bytes? What are the 
default values of DevCap.MPS and DevCtl.MPS?

Because the default value of DevCtl.MPS is not 512 bytes, it needs to be 
configured here, right?

If it's my guess, RK3588 also has the same requirements as you, just 
like the first patch I submitted.

Best regards,
Hans