On Tue, Mar 26, 2024 at 02:47:30PM +0100, Niklas Cassel wrote: > On Tue, Mar 26, 2024 at 04:40:21PM +0530, Manivannan Sadhasivam wrote: > > > > I was planning to drop enable_resources() from Qcom driver once the DBI rework > > series gets merged. Because, the resource enablement during probe is currently > > done to avoid the crash that is bound to happen if registers are accessed during > > probe. > > > > But what your observation reveals is that it is possible to get PERST# assert > > during the EP boot up itself which I was not accounting for. I always assumed > > that the EP will receive PERST# deassert first. If that is not the case, then > > this patch needs to be dropped. > > From what I saw when having debug prints from my old email to you: > https://lore.kernel.org/linux-pci/Zalu%2F%2FdNi5BhZlBU@x1-carbon/ > > > ## RC side: > # reboot > > ## EP side > [ 845.606810] pci: PERST asserted by host! > [ 852.483985] pci: PERST de-asserted by host! > [ 852.503041] pci: PERST asserted by host! > [ 852.522375] pci: link up! (LTSSM_STATUS: 0x230011) > [ 852.610318] pci: PERST de-asserted by host! > > > > So in my case, I assume that the RC asserts PERST during a SoC reset. > > This is obviously from the RC driver asserting PERST + sleep 100 ms + > PERST deassert: > [ 852.503041] pci: PERST asserted by host! > [ 852.610318] pci: PERST de-asserted by host! > > The two before that: > [ 852.483985] pci: PERST de-asserted by host! > [ 852.503041] pci: PERST asserted by host! > > appears to be because the RC I am using, incorrectly sets the PERST gpio as > ACTIVE HIGH: > https://github.com/torvalds/linux/blob/v6.9-rc1/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts#L300 > > Well, at least they are bug compatible and sets the output to: > https://github.com/torvalds/linux/blob/v6.9-rc1/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L170-L184 > > 0 and the 1, which, since the DT binding is incorrect, will actually > do the right thing and assert and the deassert PERST. > > The problem seems to be that the initial flags: > https://github.com/torvalds/linux/blob/v6.9-rc1/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L242-L243 > is: GPIOD_OUT_HIGH > > which explains why I get the extra: > [ 852.483985] pci: PERST de-asserted by host! > before > [ 852.503041] pci: PERST asserted by host! > > with basically no time between them.. > > > I guess I should send a patch to set the initial value to > GPIOD_OUT_LOW, so that the RC driver does not trigger a > "spurious" PERST deassertion when requesting the IRQ. > > > So I think this patch should be fine if the RC is not buggy, > but as we can see, in reality there are at least one platform > in mainline that does manage to get this wrong. > I've validated with x86 and Qcom RCs so far and didn't see this behavior. So I guess I'll just keep the patch for now. - Mani -- மணிவண்ணன் சதாசிவம்
