[PATCH v3 5/7] PCI: mediatek-gen3: Integrate new pwrctrl API

[Chen-Yu Tsai]

On Tue, Mar 10, 2026 at 5:50 AM Bjorn Helgaas <helgaas at kernel.org> wrote:
>
> On Mon, Mar 09, 2026 at 01:24:34PM +0800, Chen-Yu Tsai wrote:
> > On Sat, Mar 7, 2026 at 4:59 AM Bjorn Helgaas <helgaas at kernel.org> wrote:
> > > On Mon, Mar 02, 2026 at 01:31:05PM +0800, Chen-Yu Tsai wrote:
> > > > With the new PCI pwrctrl API and PCI slot binding and power drivers, we
> > > > now have a way to describe and power up WiFi/BT adapters connected
> > > > through a PCIe or M.2 slot, or exploded onto the mainboard itself.
> > ...
>
> > > So now we have:
> > >
> > >   mtk_pcie_probe
> > >     mtk_pcie_setup
> > >       mtk_pcie_startup_port
> > >         mtk_pcie_device_power_up      <-- power up
> > >         mtk_pcie_device_power_down    # error path
> > >       mtk_pcie_setup_irq              # set up controller IRQ
> > >       mtk_pcie_device_power_down      # if mtk_pcie_setup_irq() failed
> > >     pci_host_probe                    # enumerate downstream devices
> > >     mtk_pcie_device_power_down        # if pci_host_probe() failed
> > >
> > > I think this is kind of a mess because mtk_pcie_device_power_down() is
> > > called from so many places, and some of them aren't connected to the
> > > mtk_pcie_device_power_up().
> > >
> > > In mtk_pcie_setup(), mtk_pcie_setup_irq() only deals with the
> > > *controller* IRQ and has nothing to do with the downstream devices.  I
> > > think mtk_pcie_setup_irq() should be done before
> > > mtk_pcie_startup_port() so we can abort before even powering up those
> > > devices.
> >
> > Makes sense to me. I think it can even be moved outside of mtk_pcie_setup()?
> > That way we don't have mtk_pcie_irq_teardown() in two error paths?
> >
> > I can send a follow up patch for that.
>
> What if we do those cleanups first?  That would make this patch quite
> a bit simpler and we wouldn't have to undo things.  It would be a
> shame to have a complicated patch to add this stuff, then another
> complicated patch to clean it up.

Sure.

> > > In mtk_pcie_startup_port(), mtk_pcie_enable_msi() and the PCIe
> > > translation window setup also don't have anything to do with the
> > > downstream devices, so I think they should be done before
> > > calling mtk_pcie_device_power_up().  The only thing there that needs
> > > the downstream devices powered up is waiting for the link to come up.
> >
> > My guess is that if the link up fails, then mtk_pcie_startup_port() is
> > going to error out anyway, so it maybe made sense to make sure a device
> > is actually present before doing any more work.
>
> Doesn't sound very convincing to me.  mtk_pcie_set_trans_table() and
> the window setup are local things with no timeouts or anything.  I
> think the logical structure is way more important than some kind of
> hand-crafted work avoidance.

I understand. I merely meant that I have no actual knowledge of why it
happened, or if there are any concerns from the hardware side.

> Separating the PCIe controller setup from things related to the link
> and downstream devices is pretty high on my list.

Makes sense to me.

I'll send an updated version.


Thanks
ChenYu