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. > With this change, we don't need to upstream phy setting for all phys > support no_csr reset, this will save a great deal of efforts and simplify > the phy driver. Our goal is to remove proprietary PCIe firmware operations > from kernel. PHY is just the start and will be followed by controller, > clocks, regulators, etc. If program table need to be changed, the place to > do that will be UEFI. Well, that sounds like a very bad idea. Please don't do that. Linux kernel drivers should not depend on the UEFI or a bootloader. Unless there is a good reason for that, Linux should continue to be able to reset and program the PCIe PHY (as well as all other hw blocks). > > > > > Signed-off-by: Qiang Yu <quic_qianyu@xxxxxxxxxxx> > > > Signed-off-by: Wenbin Yao <quic_wenbyao@xxxxxxxxxxx> > > > --- > > > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 91 +++++++++++++++--------- > > > 1 file changed, 58 insertions(+), 33 deletions(-) > > > -- With best wishes Dmitry
