On Tuesday 19 October 2021 07:06:42 Mauro Carvalho Chehab wrote: > Before code refactor, the PERST# signals were sent at the > end of the power_on logic. Then, the PCI core would probe for > the buses and add them. > > The new logic changed it to send PERST# signals during > add_bus operation. That altered the timings. > > Also, HiKey 970 require a little more waiting time for > the PCI bridge - which is outside the SoC - to finish > the PERST# reset, and then initialize the eye diagram. Hello! Which PCIe port do you mean by PCI bridge device? Do you mean PCIe Root Port? Or upstream port on some external PCIe switch connected via PCIe bus to the PCIe Root Port? Because all of these (virtual) PCIe devices are presented as PCI bridge devices, so it is not clear to which device it refers. Normally PERST# signal is used to reset endpoint card, other end of PCIe link and so PERST# signal should not affect PCIe Root Port at all. > So, increase the waiting time for the PERST# signals to > what's required for it to also work with HiKey 970. Because PERST# signal resets endpoint card, this reset timeout should not be driver or controller specific. Mauro, if you understand this issue more deeply, could you look at my email? https://lore.kernel.org/linux-pci/20210310110535.zh4pnn4vpmvzwl5q@pali/ I think that kernel PCI subsystem does not properly handle PCIe Warm Reset and correct initialization of endpoint cards. Because similar "random PERST# timeout patches" were applied to lot of native controller drivers. PS: I'm not opposing this patch, I'm just trying to understand what is happening here and why particular number "21000" was chosen. It is defined in some standard? Or was it just randomly chosen and measures that with this number is initialization working fine? > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> > --- > > See [PATCH v14 00/11] at: https://lore.kernel.org/all/cover.1634622716.git.mchehab+huawei@xxxxxxxxxx/ > > drivers/pci/controller/dwc/pcie-kirin.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c > index de375795a3b8..bc329673632a 100644 > --- a/drivers/pci/controller/dwc/pcie-kirin.c > +++ b/drivers/pci/controller/dwc/pcie-kirin.c > @@ -113,7 +113,7 @@ struct kirin_pcie { > #define CRGCTRL_PCIE_ASSERT_BIT 0x8c000000 > > /* Time for delay */ > -#define REF_2_PERST_MIN 20000 > +#define REF_2_PERST_MIN 21000 > #define REF_2_PERST_MAX 25000 > #define PERST_2_ACCESS_MIN 10000 > #define PERST_2_ACCESS_MAX 12000 > -- > 2.31.1 >
