Received-SPF: pass (google.com: domain of linux-kernel+bounces-83741-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 27 Feb 2024 11:37:05 -0600
From: Bjorn Helgaas <helgaas@kernel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
	Rob Herring <robh@kernel.org>, Lukas Wunner <lukas@wunner.de>,
	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
Message-ID: <20240227173705.GA241732@bhelgaas>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240227170840.GR2587@thinkpad>

On Tue, Feb 27, 2024 at 10:38:40PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Feb 27, 2024 at 10:25:35AM -0600, Bjorn Helgaas wrote:
> 
> [...]
> 
> > > Ok, I got the issue. TBH, I added the device tree property based on
> > > the existing quirks for the ACPI devices. But none of the DT based
> > > platforms I'm aware of (even the legacy Qcom MSM8996 chipset
> > > released in early 2016) doesn't have any issue with D3hot. But I'm
> > > just nervous to assume it is the case for all the DT based platforms
> > > in the wild.
> > > 
> > > But to proceed further, what is your preference? Should we ammend
> > > the DT property to make it explicit that the propery only focuses on
> > > the D3hot capability of the bridge and it works as per the spec
> > > (PMCSR) or bite the bullet and enable D3hot for all the non-ACPI
> > > platforms?
> > > 
> > > We can add quirks for the bridges later on if we happen to receive
> > > any bug report.
> > 
> > I would assume all devices support D3hot via PMCSR per spec.  We can
> > add quirks if we discover something that doesn't.
> 
> When you say "all devices", are you referring to bridges in DT
> platforms or the bridges across all platforms?

This patch is only concerned with DT, so that's what I'm commenting on
here.  I don't know how to untangle the question of ACPI systems.

This patch affects platform_pci_bridge_d3(), so just based on the
"platform" in the function name, I would expect it to be concerned
with the D3cold case and whether the platform supports controlling
main power.

It looks like this patch says "we can put devices in D3cold if DT has
'supports-d3'".  But I don't know how to make sense of that because
that requires (a) platform hardware to control main power and (b)
software that knows how to use that hardware.  Wouldn't this require a
little more DT description, like "regulator X controls main power for
this bridge"?  And then an OS would only be able to actually use
D3cold if it knows how to *operate* the regulator, and it doesn't seem
like DT could answer that.

> > If we add annotations that "this device works correctly", we're
> > digging a hole for ourselves because it's impossible to remove those
> > annotations and they complicate all future maintenance.
> > 
> > Bjorn
> 
> -- 
> மணிவண்ணன் சதாசிவம்