linux-kernel - Re: [PATCH V2 5/6] i3c: mipi-i3c-hci-pci: Add optional ability to manage child runtime PM

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0goZmY08MQccrYfQPUhoGKHxv-CbQZfWSv4+O3xGMsrrQ@mail.gmail.com>
Date: Wed, 4 Feb 2026 17:16:30 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Frank Li <Frank.li@....com>
Cc: Adrian Hunter <adrian.hunter@...el.com>, alexandre.belloni@...tlin.com, 
	rafael@...nel.org, linux-i3c@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH V2 5/6] i3c: mipi-i3c-hci-pci: Add optional ability to
 manage child runtime PM

On Wed, Feb 4, 2026 at 5:06 PM Frank Li <Frank.li@....com> wrote:
>
> On Wed, Feb 04, 2026 at 01:15:10PM +0200, Adrian Hunter wrote:
> > Some platforms implement the MIPI I3C HCI Multi-Bus Instance capability,
> > where a single parent device hosts multiple I3C controller instances.  In
> > such designs, the parent - not the individual child instances - may need to
> > coordinate runtime PM so that all controllers runtime PM callbacks are
> > invoked in a controlled and synchronized manner.
> >
> > For example, if the parent enables IBI-wakeup when transitioning into a
> > low-power state, every bus instance must remain able to receive IBIs up
> > until that point.  This requires deferring the individual controllers'
> > runtime suspend callbacks (which disable bus activity) until the parent
> > decides it is safe for all instances to suspend together.
> >
> > To support this usage model:
> >
> >   * Add runtime PM and system PM callbacks in the PCI driver to invoke
> >     the mipi-i3c-hci driver's runtime PM callbacks for each instance.
> >
> >   * Introduce a driver-data flag, control_instance_pm, which opts into
> >     the new parent-managed PM behaviour.
> >
> >   * Ensure the callbacks are only used when the corresponding instance is
> >     operational at suspend time.  This is reliable because the operational
> >     state cannot change while the parent device is undergoing a PM
> >     transition, and PCI always performs a runtime resume before system
> >     suspend on current configurations, so that suspend and resume alternate
> >     irrespective of whether it is runtime or system PM.
> >
> > By that means, parent-managed runtime PM coordination for multi-instance
> > MIPI I3C HCI PCI devices is provided without altering existing behaviour on
> > platforms that do not require it.
> >
> > Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
> > ---
> ...
> > +
> > +static int mipi_i3c_hci_pci_suspend(struct device *dev)
> > +{
> > +     struct mipi_i3c_hci_pci *hci = dev_get_drvdata(dev);
> > +     struct mipi_i3c_hci_pci_pm_data pm_data = {};
> > +     int ret;
> > +
> > +     if (!hci->info->control_instance_pm)
> > +             return 0;
> > +
> > +     ret = device_for_each_child_reverse(dev, &pm_data, mipi_i3c_hci_pci_suspend_instance);
> > +     if (ret) {
> > +             if (ret == -EAGAIN || ret == -EBUSY)
> > +                     pm_runtime_mark_last_busy(&hci->pci->dev);
>
> This should not neccessary, the runtime suspend API already call it now.

I'm not sure what you mean.

This is an error path in a system suspend callback, so when exactly
runtime PM will update the last busy time?

> > +             for (int i = 0; i < pm_data.dev_cnt; i++)
> > +                     i3c_hci_rpm_resume(pm_data.dev[i]);
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static int mipi_i3c_hci_pci_resume(struct device *dev)
> > +{
> > +     struct mipi_i3c_hci_pci *hci = dev_get_drvdata(dev);
> > +     struct mipi_i3c_hci_pci_pm_data pm_data = {};
> > +     int ret;
> > +
> > +     if (!hci->info->control_instance_pm)
> > +             return 0;
> > +
> > +     ret = device_for_each_child(dev, &pm_data, mipi_i3c_hci_pci_resume_instance);
> > +     if (ret)
> > +             for (int i = 0; i < pm_data.dev_cnt; i++)
> > +                     i3c_hci_rpm_suspend(pm_data.dev[i]);
> > +
> > +     return ret;
> > +}
> > +
> >  static void mipi_i3c_hci_pci_rpm_allow(struct device *dev)
> >  {
> >       pm_runtime_put(dev);
> > @@ -323,6 +455,8 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
> >
> >  /* PM ops must exist for PCI to put a device to a low power state */
> >  static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
> > +     RUNTIME_PM_OPS(mipi_i3c_hci_pci_suspend, mipi_i3c_hci_pci_resume, NULL)
> > +     SYSTEM_SLEEP_PM_OPS(mipi_i3c_hci_pci_suspend, mipi_i3c_hci_pci_resume)
> >  };
> >
> >  static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
> > --