Re: [PATCH v2] schemas: pci: Document PCIe T_POWER_ON

[Bjorn Helgaas <helgaas@kernel.org>]

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

I don't see a driver patch that uses this yet, but I assume the driver
will use a device-specific way to set the value that will appear in
the L1 PM Substates Capabilities register, and the register visible in
config space probably is read-only as the PCIe spec describes, so I
don't think this makes the hardware non-compliant.

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

I don't have any personal experience with this hardware, but I think
the device-specific registers that back the standard PCI config
registers sometimes use different formats.  pcie-brcmstb.c is a good
example that I've tripped over several times:
https://lore.kernel.org/all/5ca0b4a2-ec3a-4fa5-a691-7e3bab88890a@broadcom.com/

Bjorn