Re: [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper

From: Bjorn Helgaas <helgaas@kernel.org>
To: Krishna Chaitanya Chundru <quic_krichai@quicinc.com>
Cc: "Stephen Boyd" <swboyd@chromium.org>,
	linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org, quic_vbadigan@quicinc.com,
	quic_hemantk@quicinc.com, quic_ramkri@quicinc.com,
	manivannan.sadhasivam@linaro.org,
	"Andy Gross" <agross@kernel.org>,
	"Bjorn Andersson" <bjorn.andersson@linaro.org>,
	"Stanimir Varbanov" <svarbanov@mm-sol.com>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>
Subject: Re: [PATCH v1] PCI: qcom: Allow L1 and its sub states on qcom dwc wrapper
Date: Wed, 15 Jun 2022 10:44:22 -0500	[thread overview]
Message-ID: <20220615154422.GA941075@bhelgaas> (raw)
In-Reply-To: <1e92745e-6fa4-a3bd-d0cb-5c1f78efbead@quicinc.com>

On Wed, Jun 15, 2022 at 06:44:19PM +0530, Krishna Chaitanya Chundru wrote:
> On 6/9/2022 3:47 AM, Stephen Boyd wrote:
> > Quoting Krishna chaitanya chundru (2022-06-03 00:18:50)
> > > Allow L1 and its sub-states in the qcom dwc pcie wrapper.
> > > By default its disabled. So enable it explicitly.
> > > 
> > Would be good to add some more details about why it's disabled by
> > default. I guess it's disabled by default in the hardware and enabling
> > it is OK to do unconditionally for all qcom dwc pcie devices?
> 
> This is disabled by default in the hardware. We can enable this for all qcom
> devices unconditionally because
> 
> Adding this patch alone will not allow aspm transitions we need to enable
> aspm configs. If particular devices doesn't want aspm
> they can disable using aspm configs.

This patch only affects qcom.  Is PCIE20_PARF_PM_CTRL qcom-specific?
Or is this something that should be done for all dwc-based drivers?

In fact, it only affects Qcom IP rev 2.7.0 and 1.9.0 (the only users
of qcom_pcie_init_2_7_0()).  I guess the other revisions don't support
ASPM L1 at all?

Does this patch affect the Link Capabilities register?  Before this
patch, does Link Cap advertise L1 support but enabling it doesn't
work?  Or does it not even advertise L1 support?

After this patch, I assume Link Cap advertises L1 support and enabling
L1 and L1 substates via PCI_EXP_LNKCTL_ASPM_L1,
PCI_L1SS_CTL1_ASPM_L1_1, and PCI_L1SS_CTL1_ASPM_L1_2 works per spec,
right?

Bjorn