linux-kernel - Re: [PATCH 2/2] phy: qcom: qmp-pcie: Add PHY register retention support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ya27ma6iah7ts6sj35payj6ek4z7m6y5v4pnxd6wtqrp3cbyta@ypvrzwa4bnfv>
Date: Wed, 22 Jan 2025 11:43:14 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: "Wenbin Yao (Consultant)" <quic_wenbyao@...cinc.com>
Cc: vkoul@...nel.org, kishon@...nel.org, p.zabel@...gutronix.de, 
	abel.vesa@...aro.org, quic_qianyu@...cinc.com, neil.armstrong@...aro.org, 
	manivannan.sadhasivam@...aro.org, quic_devipriy@...cinc.com, konrad.dybcio@....qualcomm.com, 
	linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] phy: qcom: qmp-pcie: Add PHY register retention
 support

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@...cinc.com> wrote:
> > > From: Qiang Yu <quic_qianyu@...cinc.com>
> > > 
> > > 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@...cinc.com>
> > > Signed-off-by: Wenbin Yao <quic_wenbyao@...cinc.com>
> > > ---
> > >   drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 91 +++++++++++++++---------
> > >   1 file changed, 58 insertions(+), 33 deletions(-)
> > > 

-- 
With best wishes
Dmitry