Hi Rob,
Thanks for the review.
> > +#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.
> > +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
