[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aYS-MBN5J6PrKI8s@lizhi-Precision-Tower-5810>
Date: Thu, 5 Feb 2026 10:58:40 -0500
From: Frank Li <Frank.li@....com>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: 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 V3 4/5] i3c: mipi-i3c-hci-pci: Add optional ability to
manage child runtime PM
On Thu, Feb 05, 2026 at 12:09:14PM +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>
> ---
Reviewed-by: Frank Li <Frank.Li@....com>
>
>
> Changes in V3:
>
> Remove unnecessary pm_runtime_mark_last_busy()
>
> Changes in V2:
>
> Do not enable autosuspend.
> Callbacks for parent-managed invocation were renamed
> from i3c_hci_runtime_suspend to i3c_hci_rpm_suspend and
> from i3c_hci_runtime_resume to i3c_hci_rpm_resume.
> Amend commit message slightly.
>
>
> .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 131 ++++++++++++++++++
> 1 file changed, 131 insertions(+)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index bc83caad4197..ed0efed17726 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -9,6 +9,7 @@
> #include <linux/acpi.h>
> #include <linux/bitfield.h>
> #include <linux/debugfs.h>
> +#include <linux/i3c/master.h>
> #include <linux/idr.h>
> #include <linux/iopoll.h>
> #include <linux/kernel.h>
> @@ -20,16 +21,24 @@
> #include <linux/pm_qos.h>
> #include <linux/pm_runtime.h>
>
> +#include "hci.h"
> +
> /*
> * There can up to 15 instances, but implementations have at most 2 at this
> * time.
> */
> #define INST_MAX 2
>
> +struct mipi_i3c_hci_pci_instance {
> + struct device *dev;
> + bool operational;
> +};
> +
> struct mipi_i3c_hci_pci {
> struct pci_dev *pci;
> void __iomem *base;
> const struct mipi_i3c_hci_pci_info *info;
> + struct mipi_i3c_hci_pci_instance instance[INST_MAX];
> void *private;
> };
>
> @@ -40,6 +49,7 @@ struct mipi_i3c_hci_pci_info {
> int id[INST_MAX];
> u32 instance_offset[INST_MAX];
> int instance_count;
> + bool control_instance_pm;
> };
>
> #define INTEL_PRIV_OFFSET 0x2b0
> @@ -210,6 +220,125 @@ static const struct mipi_i3c_hci_pci_info intel_si_2_info = {
> .instance_count = 1,
> };
>
> +static int mipi_i3c_hci_pci_find_instance(struct mipi_i3c_hci_pci *hci, struct device *dev)
> +{
> + for (int i = 0; i < INST_MAX; i++) {
> + if (!hci->instance[i].dev)
> + hci->instance[i].dev = dev;
> + if (hci->instance[i].dev == dev)
> + return i;
> + }
> +
> + return -1;
> +}
> +
> +#define HC_CONTROL 0x04
> +#define HC_CONTROL_BUS_ENABLE BIT(31)
> +
> +static bool __mipi_i3c_hci_pci_is_operational(struct device *dev)
> +{
> + const struct mipi_i3c_hci_platform_data *pdata = dev->platform_data;
> + u32 hc_control = readl(pdata->base_regs + HC_CONTROL);
> +
> + return hc_control & HC_CONTROL_BUS_ENABLE;
> +}
> +
> +static bool mipi_i3c_hci_pci_is_operational(struct device *dev, bool update)
> +{
> + struct mipi_i3c_hci_pci *hci = dev_get_drvdata(dev->parent);
> + int pos = mipi_i3c_hci_pci_find_instance(hci, dev);
> +
> + if (pos < 0) {
> + dev_err(dev, "%s: I3C instance not found\n", __func__);
> + return false;
> + }
> +
> + if (update)
> + hci->instance[pos].operational = __mipi_i3c_hci_pci_is_operational(dev);
> +
> + return hci->instance[pos].operational;
> +}
> +
> +struct mipi_i3c_hci_pci_pm_data {
> + struct device *dev[INST_MAX];
> + int dev_cnt;
> +};
> +
> +static bool mipi_i3c_hci_pci_is_mfd(struct device *dev)
> +{
> + return dev_is_platform(dev) && mfd_get_cell(to_platform_device(dev));
> +}
> +
> +static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
> +{
> + struct mipi_i3c_hci_pci_pm_data *pm_data = data;
> + int ret;
> +
> + if (!mipi_i3c_hci_pci_is_mfd(dev) ||
> + !mipi_i3c_hci_pci_is_operational(dev, true))
> + return 0;
> +
> + ret = i3c_hci_rpm_suspend(dev);
> + if (ret)
> + return ret;
> +
> + pm_data->dev[pm_data->dev_cnt++] = dev;
> +
> + return 0;
> +}
> +
> +static int mipi_i3c_hci_pci_resume_instance(struct device *dev, void *data)
> +{
> + struct mipi_i3c_hci_pci_pm_data *pm_data = data;
> + int ret;
> +
> + if (!mipi_i3c_hci_pci_is_mfd(dev) ||
> + !mipi_i3c_hci_pci_is_operational(dev, false))
> + return 0;
> +
> + ret = i3c_hci_rpm_resume(dev);
> + if (ret)
> + return ret;
> +
> + pm_data->dev[pm_data->dev_cnt++] = dev;
> +
> + return 0;
> +}
> +
> +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)
> + 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 +452,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[] = {
> --
> 2.51.0
>
Powered by blists - more mailing lists