All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Lukas Wunner <lukas@wunner.de>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Krzysztof Wilczy??ski <kw@linux.com>,
	Rob Herring <robh@kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	quic_krichai@quicinc.com, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v3] PCI: Add D3 support for PCI bridges in DT based platforms
Date: Tue, 5 Mar 2024 11:51:07 -0600	[thread overview]
Message-ID: <20240305175107.GA539676@bhelgaas> (raw)
In-Reply-To: <20240305162537.GA8339@thinkpad>

On Tue, Mar 05, 2024 at 09:55:37PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Feb 22, 2024 at 10:40:52AM +0100, Lukas Wunner wrote:
> > On Wed, Feb 21, 2024 at 12:20:00PM -0600, Bjorn Helgaas wrote:
> > >   1) D3hot doesn't work per spec.  This sounds like a hardware
> > >      defect in the device that should be a quirk based on
> > >      Vendor/Device ID, not something in DT.  I don't actually know if
> > >      this is common, although there are several existing quirks that
> > >      mention issues with D3.
> > 
> > My recollection is that putting Root Ports into D3hot on older x86
> > systems would raise MCEs, which is why pci_bridge_d3_possible() only
> > allows D3hot in cases which are known to work (e.g. Thunderbolt
> > controllers, machines with a recent BIOS).  It was a conservative
> > policy chosen to avoid regressions.
> 
> So pci_bridge_d3_possible() is only checking for D3hot capability?
> If so, I'd rename it to pci_bridge_d3hot_possible() and also
> 'bridge_d3' to 'bridge_d3hot' to make it explicit.

Every device is required to support D3hot (and D3cold), so I think
"d3_possible" and "d3hot_possible" are not very descriptive since they
should always be *possible*.

pci_bridge_d3_possible() seems to be more about whether hotplug and
power management events work in D3hot and maybe some firmware
coordination and validation concerns.

> Since the default value of 'd3cold_allowed' is true, I believe the
> code expects all devices to support D0 and D3cold. Please correct me
> if I'm wrong.

D3cold means "no main power", so every device "supports" that
situation.  The only time 'd3cold_allowed' can be false is when a user
has set it to false via sysfs, so I think it only reflects an
administrative policy choice.

I think the important question for the code is whether software can
remove and restore main power and maybe something about what hotplug
events or PME can be reported, and I have a really hard time following
that decision path.

Bjorn

      reply	other threads:[~2024-03-05 17:51 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-14 11:46 [PATCH v3] PCI: Add D3 support for PCI bridges in DT based platforms Manivannan Sadhasivam
2024-02-14 12:19 ` Lukas Wunner
2024-02-20 22:02 ` Bjorn Helgaas
2024-02-21  5:19   ` Manivannan Sadhasivam
2024-02-21 18:20     ` Bjorn Helgaas
2024-02-22  4:06       ` Manivannan Sadhasivam
2024-02-26 23:39         ` Bjorn Helgaas
2024-02-27  7:30           ` Manivannan Sadhasivam
2024-02-27 16:25             ` Bjorn Helgaas
2024-02-27 17:08               ` Manivannan Sadhasivam
2024-02-27 17:37                 ` Bjorn Helgaas
2024-02-27 18:40                   ` Manivannan Sadhasivam
2024-02-27 22:54                     ` Bjorn Helgaas
2024-02-22  9:40       ` Lukas Wunner
2024-02-26 23:17         ` Bjorn Helgaas
2024-03-05 16:25         ` Manivannan Sadhasivam
2024-03-05 17:51           ` Bjorn Helgaas [this message]

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=20240305175107.GA539676@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=andersson@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=konrad.dybcio@linaro.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=lukas@wunner.de \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=quic_krichai@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.