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

[neil.armstrong at linaro.org]

On 29/01/2025 12:29, Konrad Dybcio wrote:
> On 29.01.2025 9:29 AM, neil.armstrong at linaro.org 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 at quicinc.com> wrote:
>>>>>>>>> From: Qiang Yu <quic_qianyu at quicinc.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.
>>>
>>> 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