[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <589ea395-f133-4522-91e5-066c41916cd5@rock-chips.com>
Date: Fri, 14 Nov 2025 09:02:47 +0800
From: Shawn Lin <shawn.lin@...k-chips.com>
To: Manivannan Sadhasivam <mani@...nel.org>, Lukas Wunner <lukas@...ner.de>
Cc: shawn.lin@...k-chips.com,
Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>,
andersson@...nel.org, robh@...nel.org, manivannan.sadhasivam@...aro.org,
krzk@...nel.org, helgaas@...nel.org, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, lpieralisi@...nel.org, kw@...ux.com,
conor+dt@...nel.org, linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree-spec@...r.kernel.org,
quic_vbadigan@...cinc.com
Subject: Re: [PATCH v2] schemas: pci: Document PCIe T_POWER_ON
在 2025/11/14 星期五 0:41, Manivannan Sadhasivam 写道:
> On Thu, Nov 13, 2025 at 09:36:48AM +0100, Lukas Wunner wrote:
>> On Thu, Nov 13, 2025 at 09:33:54AM +0530, Krishna Chaitanya Chundru wrote:
>>> On 11/10/2025 6:11 PM, Lukas Wunner wrote:
>>>> On Mon, Nov 10, 2025 at 04:59:47PM +0530, Krishna Chaitanya Chundru wrote:
>>>>> From PCIe r6, sec 5.5.4 & Table 5-11 in sec 5.5.5 T_POWER_ON is the
>>>> Please use the latest spec version as reference, i.e. PCIe r7.0.
>>> ack.
>>>>> minimum amount of time(in us) that each component must wait in L1.2.Exit
>>>>> after sampling CLKREQ# asserted before actively driving the interface to
>>>>> ensure no device is ever actively driving into an unpowered component and
>>>>> these values are based on the components and AC coupling capacitors used
>>>>> in the connection linking the two components.
>>>>>
>>>>> This property should be used to indicate the T_POWER_ON for each Root Port.
>>>> What's the difference between this property and the Port T_POWER_ON_Scale
>>>> and T_POWER_ON_Value in the L1 PM Substates Capabilities Register?
>>>>
>>>> Why do you need this in the device tree even though it's available
>>>> in the register?
>>>
>>> This value is same as L1 PM substates value, some controllers needs to
>>> update this
>>> value before enumeration as hardware might now program this value
>>> correctly[1].
>>>
>>> [1]: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit
>>> timing
>>>
>>> <https://lore.kernel.org/all/20251104-t_power_on_fux-v1-1-eb5916e47fd7@oss.qualcomm.com/>
>>
>> Per PCIe r7.0 sec 7.8.3.2, all fields in the L1 PM Substates Capabilities
>> Register are of type "HwInit", which sec 7.4 defines as:
>>
>> "Register bits are permitted, as an implementation option, to be
>> hard-coded, initialized by system/device firmware, or initialized
>> by hardware mechanisms such as pin strapping or nonvolatile storage.
>> Initialization by system firmware is permitted only for
>> system-integrated devices.
>> Bits must be fixed in value and read-only after initialization."
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> These bits are not supposed to be writable by the operating system,
>> so what you're doing in that patch is not spec-compliant.
>>
>
> I interpret 'initialized by system/device firmware', same as 'initialized by
> OS', as both are mostly same for the devicetree platforms. So it is fine IMO.
> Ofc, if the initialization was carried out by the firmware, then OS has no
> business in changing it, but it is not the case.
>
Yes, I tend to agree with Mani. Another problem is the s2r process in
embedded ARM world. For power-saving, most platforms would cut-off the
power supply of PCIe controller during system suspend, so we save and
restore this value when relink is done, right? OS had already done this
kind of thing already.
>> I think it needs to be made explicit in the devicetree schema that
>> the property is only intended for non-compliant hardware which allows
>> (and requires) the operating system to initialize the register.
>>
>
> Sorry, I disagree. The hardware is spec compliant, just that the firmware missed
> initializing the fields.
>
>> Maybe it makes more sense to have a property which specifies the raw
>> 32-bit register contents, instead of having a property for each
>> individual field. Otherwise you'll have to amend the schema
>> whenever the PCIe spec extends the register with additional fields.
>>
>
> DT properties do not specify a register value, but instead they specify hardware
> configuration value and that's what this property is doing. The OS/other DT
> consumers should interpret this value as per the spec and program the relevant
> registers.
>
> - Mani
>
Powered by blists - more mailing lists