Re: [PATCH v6 5/9] PCI: dwc: Implement .start_link(), .stop_link() hooks

[Bjorn Helgaas <helgaas@kernel.org>]

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@oss.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