quoted

quoted

quoted

+#define MAX_MSI_HOST_IRQS	64
+#define PCIE_RESET_CONFIG_DEVICE_WAIT_MS	500

Where does this value come from?  Is there a generic value from
drivers/pci/pci.h you can use?

We check the PCIe specification to find these contents.

"With a Downstream Port that supports Link speeds greater than 5.0
GT/s, software must wait a minimum of 100 ms after Link training
completes before sending a Configuration Request to the device
immediately below that Port."

So, we think delay 500ms to let kernel issue the first configuration
command is enough after deassert PERST.

Isn't this PCIE_RESET_CONFIG_WAIT_MS?

I prefer to use #defines from the PCI core whenever possible because it makes
it easier to ensure that all drivers include the required delays.

Thank you for pointing this define.
I change to this define in next version.

quoted

quoted

quoted

+#define PCIE_RESET_CONFIG_RC_WAIT_MS		10

Ditto.  If it's an Aspeed-specific value, can you point to the
source in the Aspeed datasheet?

This delay is set to ensure that the RC internal settings are
completely reset.  We do not put its usage in our datasheet.

The "PCIE_" prefix suggests something required by the PCIe base spec.
If this is an Aspeed-specific value, perhaps remove the "PCIE_"
prefix?

Agreed.

quoted

quoted

quoted

+static int aspeed_ast2700_child_config(struct pci_bus *bus,
+unsigned int

devfn,

quoted

+				       int where, int size, u32 *val,
+				       bool write)
+{
+	struct aspeed_pcie *pcie = bus->sysdata;
+	u32 bdf_offset, status, cfg_val;
+	int ret;
+
+	bdf_offset = aspeed_pcie_get_bdf_offset(bus, devfn, where);
+
+	cfg_val = CRG_PAYLOAD_SIZE;
+	if (write)
+		cfg_val |= (bus->number == 1) ? CRG0_WRITE_FMTTYPE :

CRG1_WRITE_FMTTYPE;

quoted

+	else
+		cfg_val |= (bus->number == 1) ? CRG0_READ_FMTTYPE :
+CRG1_READ_FMTTYPE;

I don't think you should assume that bus 0 is the root bus.  The
root bus number should come from the DT bus-range.

Just making sure you saw this part since you didn't mention it.

Agreed.
I have checked our design and verified in different root bus number.
I will modify this part in next version.

Thanks,
Jacky