Hi Rob,

Thanks for the review.

quoted

+#define CORE_RC_PHYIF_CTL              0x00024
+#define   CORE_RC_PHYIF_CTL_RUN                BIT(0)
+#define CORE_RC_PHYIF_STAT             0x00028
+#define   CORE_RC_PHYIF_STAT_REFCLK    BIT(4)
+#define CORE_RC_CTL                    0x00050
+#define   CORE_RC_CTL_RUN              BIT(0)
+#define CORE_RC_STAT                   0x00058
+#define   CORE_RC_STAT_READY           BIT(0)
+#define CORE_FABRIC_STAT               0x04000
+#define   CORE_FABRIC_STAT_MASK                0x001F001F
+#define CORE_PHY_CTL                   0x80000
+#define   CORE_PHY_CTL_CLK0REQ         BIT(0)
+#define   CORE_PHY_CTL_CLK1REQ         BIT(1)
+#define   CORE_PHY_CTL_CLK0ACK         BIT(2)
+#define   CORE_PHY_CTL_CLK1ACK         BIT(3)
+#define   CORE_PHY_CTL_RESET           BIT(7)

I was going to say these should be a phy driver perhaps, but they are
unused. So for now, just drop them.

Removed in v2.

CORE_PHY_CTRL is used in the asahi linux bootloader (m1n1, shared between
linux+uboot+bsd) to do early pcie bringup. They are indeed not used
here, nor are they used in the uboot/bsd drivers.

quoted

+static int apple_pcie_setup_port(struct apple_pcie *pcie, unsigned int i)
+{
+       struct fwnode_handle *fwnode = dev_fwnode(pcie->dev);

Doesn't look like you ever use the fwnode, just get the DT node
pointer. Unless this driver is going to use ACPI someday (and ACPI
changes how PCI is done), there's no point in using fwnode.

Dropped in v2.

That was a copypaste fail splitting off apple_pcie_setup_port from
apple_msi_init in an early revision.

It's preferred to use platform resource api and ioremap over DT functions.
...
Use devm_platform_ioremap_resource instead.

Done in v2.

Thanks,

Alyssa