On Mon, Jul 24, 2023 at 06:48:47PM +0800, Kevin Xie wrote: > On 2023/7/21 0:15, Bjorn Helgaas wrote: > > On Thu, Jul 20, 2023 at 06:11:59PM +0800, Kevin Xie wrote: > >> On 2023/7/20 0:48, Bjorn Helgaas wrote: > >> > On Wed, Jul 19, 2023 at 06:20:56PM +0800, Minda Chen wrote: > >> >> Add StarFive JH7110 SoC PCIe controller platform > >> >> driver codes. > >> However, in the compatibility testing with several NVMe SSD, we > >> found that Lenovo Thinklife ST8000 NVMe can not get ready in 100ms, > >> and it actually needs almost 200ms. Thus, we increased the T_PVPERL > >> value to 300ms for the better device compatibility. > > ... > > > > Thanks for this valuable information! This NVMe issue potentially > > affects many similar drivers, and we may need a more generic fix so > > this device works well with all of them. > > > > T_PVPERL is defined to start when power is stable. Do you have a way > > to accurately determine that point? I'm guessing this: > > > > gpiod_set_value_cansleep(pcie->power_gpio, 1) > > > > turns the power on? But of course that doesn't mean it is instantly > > stable. Maybe your testing is telling you that your driver should > > have a hardware-specific 200ms delay to wait for power to become > > stable, followed by the standard 100ms for T_PVPERL? > > You are right, we did not take the power stable cost into account. > T_PVPERL is enough for Lenovo Thinklife ST8000 NVMe SSD to get ready, > and the extra cost is from the power circuit of a PCIe to M.2 connector, > which is used to verify M.2 SSD with our EVB at early stage. Hmm. That sounds potentially interesting. I assume you're talking about something like this: https://www.amazon.com/dp/B07JKH5VTL I'm not familiar with the timing requirements for something like this. There is a PCIe M.2 spec with some timing requirements, but I don't know whether or how software is supposed to manage this. There is a T_PVPGL (power valid to PERST# inactive) parameter, but it's implementation specific, so I don't know what the point of that is. And I don't see a way for software to even detect the presence of such an adapter. But I assume some end users will use adapters like this and expect it to "just work," so it would be nice if it did. > As the Thinklife NVMe SSD may be a halted product, and the onboard > power circuit of VisionFive V2 is no problem, we decided revert the > sleep time to be 100ms. Even though the product may be end-of-life, people will probably still try to use it, and I would like it to work. Otherwise we end up with frustrated users and problem reports that are hard to resolve. But I don't know where to go here. Bjorn