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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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...


If whatever sits in the middle *must* hard-reset the phy, it can save the
register state beforehand and restore them after the reset

Yes, we should make use the noscr if the PHY is always programmed, but we should be
always able to reprogram the PHY entirely to recover a faulty programmation.

We aren't considering any possibility of faulty programming - it's either
programmed, or not. And if the values configured by the bootloader are wrong,
the device's firmware is considered faulty.

Most devices probably follow the exact same magic values as our reference
boards (though these values relate to analog characteristics, so perhaps not
*all* of them, which is another argument for keeping the BL state) and these
are extensively tested internally before any production devices make it out
the door. Any updates deep into the product life are most likely just "nice
to have"s and not anything critical, and as I've mentioned, we can still have
overrides with the current logic inside this driver.

Konrad





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux