On 29.01.2025 3:19 PM, neil.armstrong@xxxxxxxxxx wrote: > On 29/01/2025 14:55, Konrad Dybcio wrote: >> On 29.01.2025 2:41 PM, neil.armstrong@xxxxxxxxxx wrote: >>> On 29/01/2025 12:29, Konrad Dybcio wrote: >>>> On 29.01.2025 9:29 AM, neil.armstrong@xxxxxxxxxx wrote: >>>>> On 25/01/2025 14:10, Konrad Dybcio wrote: >>>>>> On 24.01.2025 8:08 AM, Manivannan Sadhasivam wrote: >>>>>>> + Mayank (with whom I discussed this topic internally) >>>>>>> >>>>>>> On Fri, Jan 24, 2025 at 02:22:01PM +0800, Qiang Yu wrote: >>>>>>>> >>>>>>>> On 1/22/2025 5:43 PM, Dmitry Baryshkov wrote: >>>>>>>>> On Wed, Jan 22, 2025 at 03:17:39PM +0800, Wenbin Yao (Consultant) wrote: >>>>>>>>>> On 1/21/2025 6:36 PM, Dmitry Baryshkov wrote: >>>>>>>>>>> On Tue, 21 Jan 2025 at 11:43, Wenbin Yao <quic_wenbyao@xxxxxxxxxxx> wrote: >>>>>>>>>>>> From: Qiang Yu <quic_qianyu@xxxxxxxxxxx> >>>>>>>>>>>> >>>>>>>>>>>> Currently, BCR reset and PHY register setting are mandatory for every port >>>>>>>>>>>> before link training. However, some QCOM PCIe PHYs support no_csr reset. >>>>>>>>>>>> Different than BCR reset that is used to reset entire PHY including >>>>>>>>>>>> hardware and register, once no_csr reset is toggled, only PHY hardware will >>>>>>>>>>>> be reset but PHY registers will be retained, >>>>>>>>>>> I'm sorry, I can't parse this. >>>>>>>>>> The difference between no_csr reset and bcr reset is that no_csr reset >>>>>>>>>> doesn't reset the phy registers. If a phy is enabled in UEFI, its registers >>>>>>>>>> are programed. After Linux boot up, the registers will not be reset but >>>>>>>>>> keep the value programmed by UEFI if we only do no_csr reset, so we can >>>>>>>>>> skip phy setting. >>>>>>>>> Please fix capitalization of the abbreviations (PHY, BCR) and add >>>>>>>>> similar text to the commit message. >>>>>>>>> >>>>>>>>>>>> which means PHY setting can >>>>>>>>>>>> be skipped during PHY init if PCIe link was enabled in booltloader and only >>>>>>>>>>>> no_csr is toggled after that. >>>>>>>>>>>> >>>>>>>>>>>> Hence, determine whether the PHY has been enabled in bootloader by >>>>>>>>>>>> verifying QPHY_START_CTRL register. If it is programmed and no_csr reset is >>>>>>>>>>>> present, skip BCR reset and PHY register setting, so that PCIe link can be >>>>>>>>>>>> established with no_csr reset only. >>>>>>>>>>> This doesn't tell us why we want to do so. The general rule is not to >>>>>>>>>>> depend on the bootloaders at all. The reason is pretty simple: it is >>>>>>>>>>> hard to update bootloaders, while it is relatively easy to update the >>>>>>>>>>> kernel. If the hardware team issues any kind of changes to the >>>>>>>>>>> programming tables, the kernel will get them earlier than the >>>>>>>>>>> bootloader. >>>>>> >>>>>> We're assuming that if a product has shipped, the sequences used to power up >>>>>> the PHY in the bootloader (e.g. for NVMe) are already good. >>>>>> >>>>>> If some tragedy happens and an erratum is needed, we can always introduce a >>>>>> small override with the existing driver infrastructure (i.e. adding a new >>>>>> entry with a couple registers worth of programming sequence, leaving the other >>>>>> values in tact) >>>>> >>>>> Assuming Linux will be always ran directly after the bootloader is a wild assumption. >>>> >>>> Situations like >>>> >>>> [normal boot chain] -> [... (resets the PHY and doesn't reprogram it)] -> Linux >>>> >>>> are both so unlikely and so intentional-by-the-user that it doesn't seem >>>> worth considering really. >>> >>> In embedded/mobile/edge world, definitely, in compute/PC-like market, not really. >>> >>> You'll have people add some custom bootloaders, hypervisors, who knows what... >> >> I see, however you actually have to intentionally assert the non-NO_CSR PHY >> reset from said custom bootloaders, hypervisors and whoknowswhats for the >> programmed sequence to be erased. So I have no idea what the issue is here. > > I won't argue further, but you know as I do that relying on the bootloader state > is a dangerous game, and we already rely a lot for dsp stuff and we have > a lot lot of issue related to the UEFI implementation already on production > devices. > > I'm not against the nocsr stuff, which can be a big win for boot time, but > honestly not adding a few registers in table seems risky enough, and we should > probably delay this experiment until we are sure the nocsr stuff works fine. I tested a range of mobile/compute platforms and only the latter kept the PCIe PHYs initialized after dropping to the OS. No adverse effects that I can tell. Konrad