Message-ID: <zlogycfwvdclx2wpxyzhdm7m5edsckcfscz4ddor5seyhmiyf4@kd7ueiga6aaq>
Date: Mon, 22 Sep 2025 22:03:12 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: manivannan.sadhasivam@....qualcomm.com, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Saravana Kannan <saravanak@...gle.com>, 
	linux-pci@...r.kernel.org, linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>, 
	Brian Norris <briannorris@...omium.org>
Subject: Re: [PATCH v3 4/4] PCI: qcom: Allow pwrctrl core to control PERST#
 if 'reset-gpios' property is available

On Mon, Sep 22, 2025 at 11:00:41AM -0500, Bjorn Helgaas wrote:
> On Fri, Sep 19, 2025 at 01:45:51PM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Sep 18, 2025 at 01:53:56PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Sep 17, 2025 at 03:53:25PM +0530, Manivannan Sadhasivam wrote:
> > > > On Tue, Sep 16, 2025 at 03:48:10PM GMT, Bjorn Helgaas wrote:
> > > > > On Fri, Sep 12, 2025 at 02:05:04PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
> > > > > > 
> > > > > > For historic reasons, the pcie-qcom driver was controlling the
> > > > > > power supply and PERST# GPIO of the PCIe slot.
> > > > > 
> > > > > > This turned out to be an issue as the power supply requirements
> > > > > > differ between components. For instance, some of the WLAN
> > > > > > chipsets used in Qualcomm systems were connected to the Root
> > > > > > Port in a non-standard way using their own connectors.
> > > > > 
> > > > > This is kind of hand-wavy.  I don't know what a non-standard
> > > > > connector has to do with this.  I assume there's still a PCIe link
> > > > > from Root Port to WLAN, and there's still a PERST# signal to the
> > > > > WLAN device and a Root Port GPIO that asserts/deasserts it.
> > > > 
> > > > If we have a non-standard connector, then the power supply
> > > > requirements change.  There is no longer the standard 3.3v, 3.3Vaux,
> > > > 1.8v supplies, but plenty more.  For instance, take a look at the
> > > > WCN6855 WiFi/BT combo chip in the Lenovo X13s laptop:
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts#n414
> > > > 
> > > > These supplies directly go from the host PMIC to the WCN6855 chip
> > > > integrated in the PCB itself. And these supplies need to be turned
> > > > on/off in a sequence also, together with the EN/SWCTRL GPIOs, while
> > > > sharing with the Bluetooth driver.
> > > 
> > > It sounds like the WCN6855 power supplies have nothing to do with the
> > > qcom PCIe controller, the Root Port, or any switches leading to the
> > > WCN6855.  And I guess the same for the wlan-enable, bt-enable, and
> > > swctrl GPIOs?
> > > 
> > >   wcn6855-pmu {
> > >           compatible = "qcom,wcn6855-pmu";
> > >           wlan-enable-gpios = <&tlmm 134 GPIO_ACTIVE_HIGH>;
> > >           bt-enable-gpios = <&tlmm 133 GPIO_ACTIVE_HIGH>;
> > >           swctrl-gpios = <&tlmm 132 GPIO_ACTIVE_HIGH>;
> > >           regulators {
> > >                   vreg_pmu_rfa_cmn_0p8: ldo0 {
> > >                           regulator-name = "vreg_pmu_rfa_cmn_0p8";
> > >                   ...
> > > 
> > >   &pcie4_port0 {
> > >           wifi@0 {
> > >                   compatible = "pci17cb,1103";
> > >                   vddrfacmn-supply = <&vreg_pmu_rfa_cmn_0p8>;
> > >                   ...
> > > 
> > > But I guess PERST# isn't described in the same place (not in
> > > wcn6855-pmu)?  Looks like maybe it's this, which IIUC is part of the
> > > pcie4 host bridge?
> > > 
> > >   &pcie4 {
> > >           max-link-speed = <2>;
> > >           perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
> > >           wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
> > > 
> > > Does that mean this PERST# signal is driven by a GPIO and routed
> > > directly to the WCN6855?  Seems like there's some affinity between the
> > > WCN6855 power supplies and the WCN6855 PERST# signal, and maybe they
> > > would be better described together?
> > 
> > Yes, 'perst-gpios' is the PERST# signal that goes from the host
> > system to the WCN6855 chip. But we cannot define this signal in the
> > WCN6855 node as the DT binding only allows to define it in the PCI
> > bridge nodes. This is why it is currently defined in the host bridge
> > node. But when this platform switches to the per-Root Port binding,
> > this property will be moved to the Root Port node as 'reset-gpios'.
> 
> I'm questioning what the right place is to describe PERST#.  Neither
> the host bridge/Root Complex nor the Root Port has any architected
> support for asserting PERST#, so we can't write generic code to handle
> it.
> 

True.

> The PERST# signal is defined by the CEM specs, so can be physically
> included in a standard connector or cable that carries the Link.  The
> Link is originated by a Downstream Port, and the PCIe spec tells us
> how to operate the Link using the DP's Link Control, Link Status, etc.
> 
> But PERST# might not originate in the Downstream Port, and the spec
> doesn't tell us how to assert/deassert it, so I'm not sure it really
> fits in the same class as things like 'max-link-speed' and
> 'num-lanes'.  Maybe it doesn't need to be in either the host bridge or
> the Root Port?
> 

While I agree that PERST# has nothing to do with the Downstream Port, we don't
have any better way to represent it in devicetree. Either this has to be defined
in Host Bridge or Root Port/Bridge or Endpoint node. Currently, the devicetree
spec allows it to be defined in both Host Bridge and Root Port nodes, but not in
the Endpoint node. AFAIU, this is due to the fact that PERST# is a host
controlled signal, not device (unlike WAKE#). So we cannot put it in the
Endpoint node.

Moreover, if it is defined in the Host Bridge node, then we cannot do
PERST#<->device mapping in the case of multiple PERST# signals. So defining it
in the Root Port/Bridge node seemed to be the ideal place (till when there is a
single PERST# per slot/downstream port).

Maybe Rob could share more of the insight.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists