All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Cc: andersson@kernel.org, robh@kernel.org,
	manivannan.sadhasivam@linaro.org, krzk@kernel.org,
	helgaas@kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, lpieralisi@kernel.org, kw@linux.com,
	conor+dt@kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree-spec@vger.kernel.org,
	quic_vbadigan@quicinc.com
Subject: Re: [PATCH v2] schemas: pci: Document PCIe T_POWER_ON
Date: Thu, 13 Nov 2025 09:36:48 +0100	[thread overview]
Message-ID: <aRWYoHvaCCN95ZR9@wunner.de> (raw)
In-Reply-To: <44c7b4a8-33ce-4516-81bf-349b5e555806@oss.qualcomm.com>

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

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.

Thanks,

Lukas

  reply	other threads:[~2025-11-13  8:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-10 11:29 [PATCH v2] schemas: pci: Document PCIe T_POWER_ON Krishna Chaitanya Chundru
2025-11-10 12:03 ` Manivannan Sadhasivam
2025-11-13  7:48   ` Krishna Chaitanya Chundru
2025-11-10 12:41 ` Lukas Wunner
2025-11-13  4:03   ` Krishna Chaitanya Chundru
2025-11-13  8:36     ` Lukas Wunner [this message]
2025-11-13 15:54       ` Bjorn Helgaas
2025-11-13 16:41       ` Manivannan Sadhasivam
2025-11-14  1:02         ` Shawn Lin
2025-11-13  4:31   ` Manivannan Sadhasivam
2025-11-13  5:16     ` Anand Moon
2025-11-13  5:50       ` Manivannan Sadhasivam
2025-11-13  6:16         ` Anand Moon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aRWYoHvaCCN95ZR9@wunner.de \
    --to=lukas@wunner.de \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree-spec@vger.kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=helgaas@kernel.org \
    --cc=krishna.chundru@oss.qualcomm.com \
    --cc=krzk@kernel.org \
    --cc=kw@linux.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=quic_vbadigan@quicinc.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.