Hello! On Wednesday 17 November 2021 14:12:45 Bjorn Helgaas wrote: > [+cc Pali] > > On Wed, Nov 17, 2021 at 04:00:53PM +0000, Marc Zyngier wrote: > > While the Apple PCIe driver works correctly when directly booted > > from the firmware, it fails to initialise when the kernel is booted > > from a bootloader using PCIe such as u-boot. > > > > That's beacuse we're missing a proper reset of the port (we only > > clear the reset, but never assert it). > > s/beacuse/because/ > > > Bring the port back to life by wiggling the #PERST pin for 100ms > > (as per the spec). > > I cc'd Pali because I think he's interested in consolidating or > somehow rationalizing delays like this. > > If we have a specific spec reference here, I think it would help that > effort. I *think* it's PCIe r5.0, sec 6.6.1, which mentions the 100ms > along with some additional constraints, like waiting 100ms after Link > training completes for ports that support > 5.0 GT/s, whether > Readiness Notifications are used, and CRS Software Visiblity. This is not 100ms timeout "after link training completes". Timeout in this patch is between flipping PERST# signal, so timeout means how long needs to be endpoint card in reset state. And this timeout cannot be controller specific. In past I have tried to find this timeout in specifications, I was not able. Some summary is in my email: https://lore.kernel.org/linux-pci/20210310110535.zh4pnn4vpmvzwl5q@pali/ So I would like to know, why was chosen 100ms for msleep() in this patch? > > Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> > > Cc: Alyssa Rosenzweig <alyssa@xxxxxxxxxxxxx> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > --- > > drivers/pci/controller/pcie-apple.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > > index 1bf4d75b61be..bbea5f6e0a68 100644 > > --- a/drivers/pci/controller/pcie-apple.c > > +++ b/drivers/pci/controller/pcie-apple.c > > @@ -543,6 +543,9 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie, > > if (ret < 0) > > return ret; > > > > + /* Hold #PERST for 100ms as per the spec */ > > + gpiod_set_value(reset, 0); > > + msleep(100); > > rmw_set(PORT_PERST_OFF, port->base + PORT_PERST); > > gpiod_set_value(reset, 1); > > > > -- > > 2.30.2 > >