linux-kernel - Re: [PATCH v6 5/9] PCI: dwc: Implement .start_link(), .stop

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250925172517.GA2169496@bhelgaas>
Date: Thu, 25 Sep 2025 12:25:17 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Manivannan Sadhasivam <mani@...nel.org>
Cc: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	chaitanya chundru <quic_krichai@...cinc.com>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konradybcio@...nel.org>,
	cros-qcom-dts-watchers@...omium.org,
	Jingoo Han <jingoohan1@...il.com>,
	Bartosz Golaszewski <brgl@...ev.pl>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, quic_vbadigan@...cnic.com,
	amitk@...nel.org, linux-pci@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-msm@...r.kernel.org, jorge.ramirez@....qualcomm.com,
	linux-arm-kernel@...ts.infradead.org,
	Dmitry Baryshkov <lumag@...nel.org>
Subject: Re: [PATCH v6 5/9] PCI: dwc: Implement .start_link(), .stop_link()
 hooks

On Thu, Sep 25, 2025 at 09:49:16PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Sep 25, 2025 at 09:54:16AM -0500, Bjorn Helgaas wrote:
> > On Thu, Aug 28, 2025 at 05:39:02PM +0530, Krishna Chaitanya Chundru wrote:
> > > Implement stop_link() and  start_link() function op for dwc drivers.
> > > 
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware-host.c | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index 952f8594b501254d2b2de5d5e056e16d2aa8d4b7..bcdc4a0e4b4747f2d62e1b67bc1aeda16e35acdd 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -722,10 +722,28 @@ void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, unsigned int devfn,
> > >  }
> > >  EXPORT_SYMBOL_GPL(dw_pcie_own_conf_map_bus);
> > >  
> > > +static int dw_pcie_op_start_link(struct pci_bus *bus)
> > > +{
> > > +	struct dw_pcie_rp *pp = bus->sysdata;
> > > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > +
> > > +	return dw_pcie_host_start_link(pci);
> > 
> > This takes a pci_bus *, which could be any PCI bus, but this only
> > works for root buses because it affects the link from a Root Port.
> > 
> > I know the TC9563 is directly below the Root Port in the current
> > topology, but it seems like the ability to configure a Switch with
> > I2C or similar is potentially of general interest, even if the
> > switch is deeper in the hierarchy.
> > 
> > Is there a generic way to inhibit link training, e.g., with the
> > Link Disable bit in the Link Control register?  If so, this could
> > potentially be done in a way that would work for any vendor and
> > for any Downstream Port, including Root Ports and Switch
> > Downstream Ports.
> 
> FWIW, the link should not be stopped for a single device, since it
> could affect other devices in the bus. Imagine if this switch is
> connected to one of the downstream port of another switch. Then
> stopping and starting the link will affect other devices connected
> to the upstream switch as well.

Link Disable would affect all devices downstream of the bridge where
it is set, same as dw_pcie_op_stop_link().

> This driver is doing it right now just because, there is no other
> way to control the switch state machine. Ideally, we would want the
> PERST# to be in asserted stage to keep the device from starting the
> state machine, then program the registers over I2C and deassert
> PERST#. This will work across all of the host controller drivers (if
> they support pwrctrl framework).

I don't think there's a way to implement .start_link() and
.stop_link() for ACPI unless it's by using Link Disable, which is why
I asked about this.  If Link Disable *does* work, it would be a very
generic way to do this because it's part of the PCIe base spec.

Bjorn