On Thu, Oct 30, 2025 at 01:16:12PM +0100, Thierry Reding wrote:

On Wed, Oct 29, 2025 at 12:46:54PM -0500, Bjorn Helgaas wrote:

quoted

On Wed, Oct 29, 2025 at 05:33:31PM +0100, Thierry Reding wrote:

quoted

From: Thierry Reding <redacted>

Pass the driver-specific data via the syscore struct and use it in the
syscore ops.

quoted

quoted

+++ b/arch/mips/pci/pci-alchemy.c

@@ -33,6 +33,7 @@
 
 struct alchemy_pci_context {
 	struct pci_controller alchemy_pci_ctrl; /* leave as first member! */
+	struct syscore syscore;
 	void __iomem *regs;			/* ctrl base */
 	/* tools for wired entry for config space access */
 	unsigned long last_elo0;

@@ -46,12 +47,6 @@ struct alchemy_pci_context {
 	int (*board_pci_idsel)(unsigned int devsel, int assert);
 };
 
-/* for syscore_ops. There's only one PCI controller on Alchemy chips, so this
- * should suffice for now.
- */
-static struct alchemy_pci_context *__alchemy_pci_ctx;
-
-
 /* IO/MEM resources for PCI. Keep the memres in sync with fixup_bigphys_addr
  * in arch/mips/alchemy/common/setup.c
  */

@@ -306,9 +301,7 @@ static int alchemy_pci_def_idsel(unsigned int devsel, int assert)
 /* save PCI controller register contents. */
 static int alchemy_pci_suspend(void *data)
 {
-	struct alchemy_pci_context *ctx = __alchemy_pci_ctx;
-	if (!ctx)
-		return 0;
+	struct alchemy_pci_context *ctx = data;
 
 	ctx->pm[0]  = __raw_readl(ctx->regs + PCI_REG_CMEM);
 	ctx->pm[1]  = __raw_readl(ctx->regs + PCI_REG_CONFIG) & 0x0009ffff;

@@ -328,9 +321,7 @@ static int alchemy_pci_suspend(void *data)
 
 static void alchemy_pci_resume(void *data)
 {
-	struct alchemy_pci_context *ctx = __alchemy_pci_ctx;
-	if (!ctx)
-		return;
+	struct alchemy_pci_context *ctx = data;
 
 	__raw_writel(ctx->pm[0],  ctx->regs + PCI_REG_CMEM);
 	__raw_writel(ctx->pm[2],  ctx->regs + PCI_REG_B2BMASK_CCH);

@@ -359,10 +350,6 @@ static const struct syscore_ops alchemy_pci_syscore_ops = {
 	.resume = alchemy_pci_resume,
 };
 
-static struct syscore alchemy_pci_syscore = {
-	.ops = &alchemy_pci_syscore_ops,
-};
-
 static int alchemy_pci_probe(struct platform_device *pdev)
 {
 	struct alchemy_pci_platdata *pd = pdev->dev.platform_data;

@@ -480,9 +467,10 @@ static int alchemy_pci_probe(struct platform_device *pdev)
 	__raw_writel(val, ctx->regs + PCI_REG_CONFIG);
 	wmb();
 
-	__alchemy_pci_ctx = ctx;
 	platform_set_drvdata(pdev, ctx);
-	register_syscore(&alchemy_pci_syscore);
+	ctx->syscore.ops = &alchemy_pci_syscore_ops;
+	ctx->syscore.data = ctx;
+	register_syscore(&ctx->syscore);

As far as I can tell, the only use of syscore in this driver is for
suspend/resume.

This is a regular platform_device driver, so instead of syscore, I
think it should use generic power management like other PCI host
controller drivers do, something like this:

  static int alchemy_pci_suspend_noirq(struct device *dev)
  ...

  static int alchemy_pci_resume_noirq(struct device *dev)
  ...

  static DEFINE_NOIRQ_DEV_PM_OPS(alchemy_pci_pm_ops,
                                 alchemy_pci_suspend_noirq,
                                 alchemy_pci_resume_noirq);

  static struct platform_driver alchemy_pcictl_driver = {
          .probe          = alchemy_pci_probe,
          .driver = {
                  .name   = "alchemy-pci",
                  .pm     = pm_sleep_ptr(&alchemy_pci_pm_ops),
          },
  };

Here's a sample in another driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/cadence/pci-j721e.c?id=v6.17#n663

I thought so too, but then I looked at the history and saw that it was
initially regular PM ops and then fixed by using syscore in this commit:

    commit 864c6c22e9a5742b0f43c983b6c405d52817bacd
    Author: Manuel Lauss [off-list ref]
    Date:   Wed Nov 16 15:42:28 2011 +0100
    
        MIPS: Alchemy: Fix PCI PM
    
        Move PCI Controller PM to syscore_ops since the platform_driver PM methods
        are called way too late on resume and far too early on suspend (after and
        before PCI device resume/suspend).
        This also allows to simplify wired entry management a bit.
    
        Signed-off-by: Manuel Lauss [off-list ref]
        Cc: linux-mips@linux-mips.org
        Patchwork: https://patchwork.linux-mips.org/patch/3007/
        Signed-off-by: Ralf Baechle [off-list ref]

The alchemy PCI controller is a platform_device, and it must be
initialized before enumerating the PCI devices below it.  The same
order should apply for suspend/resume (suspend PCI devices, then PCI
controller; resume PCI controller, then PCI devices).

So if this didn't work before, I think it means something is messed up
with the device hierarchy.

But I understand the difficulty of testing changes here, so syscore is
simplest from that point of view.

It does complicate maintenance though.  I think all of mips ultimately
uses register_pci_controller() and pcibios_scanbus().  Neither really
contains anything mips-specific, so they duplicate a lot of the code
in pci_host_probe().  Oh well, I guess that's part of the burden of
supporting old platforms forever.

Bjorn