On Wed, Oct 29, 2025 at 12:46:54PM -0500, Bjorn Helgaas wrote: > On Wed, Oct 29, 2025 at 05:33:31PM +0100, Thierry Reding wrote: > > From: Thierry Reding <[email protected]> > > > > Pass the driver-specific data via the syscore struct and use it in the > > syscore ops. > > Would be nice to include the "instead of global variable" part here so > the commit log includes the benefit and can stand alone even without > the subject.
Good point.
> Awesome to get rid of another global variable! More comments below.
\o/
> > +++ 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 <[email protected]>
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 <[email protected]>
Cc: [email protected]
Patchwork: https://patchwork.linux-mips.org/patch/3007/
Signed-off-by: Ralf Baechle <[email protected]>
So unfortunately I don't think it'll work for this driver.
Thierry
signature.asc
Description: PGP signature
