Message-ID: <a8a608c8-c6ce-415b-ac02-db1246edb15e@intel.com>
Date: Fri, 30 Jan 2026 09:00:33 +0200
From: Adrian Hunter <adrian.hunter@...el.com>
To: Frank Li <Frank.li@....com>
CC: <alexandre.belloni@...tlin.com>, <linux-i3c@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>, <linux-pm@...r.kernel.org>
Subject: Re: [PATCH 5/7] i3c: mipi-i3c-hci: Allow parent to manage runtime PM

On 29/01/2026 23:00, Frank Li wrote:
> On Thu, Jan 29, 2026 at 10:28:14PM +0200, Adrian Hunter wrote:
>> On 29/01/2026 22:00, Frank Li wrote:
>>> On Thu, Jan 29, 2026 at 08:18:39PM +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 enter low-power states
>>>> together, and all runtime suspend callbacks are invoked in a controlled
>>>> and synchronized manner.
>>>>
>>>> For example, if the parent enables IBI-wakeup when transitioning into a
>>>> low-power state,
>>>
>>> Does your hardware support recieve IBI when runtime suspend?
>>
>> When runtime suspended (in D3), the hardware first triggers a Power Management
>> Event (PME) when the SDA line is pulled low to signal the START condition of an IBI.
>> The PCI subsystem will then runtime-resume the device.  When the bus is enabled,
>> the clock is started and the IBI is received.
> 
> It align my assumption, why need complex solution.
> 
> SDA->PME->IRQ should handle by hardware, so irq handle queue IBI to working
> queue.
> 
> IBI work will try do transfer, which will call runtime resume(), then
> transfer data.
> 
> What's issue?

The PME indicates I3C START (SDA line pulled low).  The controller is
in a low power state unable to operate the bus.  At this point it is not
known what I3C device has pulled down the SDA line, or even if it is an
IBI since it is indistinguishable from hot-join at this point.

The PCI PME IRQ is not the device's IRQ.  It is handled by acpi_irq()
which ultimately informs the PCI subsystem to wake the PCI device.
The PCI subsystem performs pm_request_resume(), refer pci_acpi_wake_dev().

When the controller is resumed, it enables the I3C bus and the IBI is
finally delivered normally.

However, none of that is related to this patch.

This patch is because the PCI device has 2 I3C bus instances and only 1 PME
wakeup.  The PME becomes active when the PCI device is put to a low
power state.  Both I3C bus instances must be runtime suspended then.
Similarly, upon resume the PME is no longer active, so both I3C bus instances
must make their buses operational - we don't know which may have received
an IBI.  And there may be further IBIs which can't be received unless the
associated bus is operational.  The PCI device is no longer in a low power
state, so there will be no PME in that case.

> 
> Frank
> 
>>
>>>
>>> Frank
>>>
>>>> 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:
>>>>
>>>>   * Export the controller's runtime PM suspend/resume callbacks so that
>>>>     the parent can invoke them directly.
>>>>
>>>>   * Add a new quirk, HCI_QUIRK_RPM_PARENT_MANAGED, which designates the
>>>>     parent device as the controller’s runtime PM device (rpm_dev).  When
>>>>     used without HCI_QUIRK_RPM_ALLOWED, this also prevents the child
>>>>     instance’s system-suspend callbacks from using
>>>>     pm_runtime_force_suspend()/pm_runtime_force_resume(), since runtime
>>>>     PM is managed entirely by the parent.
>>>>
>>>>   * Move DEFAULT_AUTOSUSPEND_DELAY_MS into the header so it can be shared
>>>>     by parent-managed PM implementations.
>>>>
>>>> The new quirk allows platforms with multi-bus parent-managed PM
>>>> infrastructure to correctly coordinate runtime PM across all I3C HCI
>>>> instances.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
>>>> ---
>>>>  drivers/i3c/master/mipi-i3c-hci/core.c | 25 ++++++++++++++++---------
>>>>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  6 ++++++
>>>>  2 files changed, 22 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
>>>> index ec4dbe64c35e..cb974b0f9e17 100644
>>>> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
>>>> @@ -733,7 +733,7 @@ static int i3c_hci_reset_and_init(struct i3c_hci *hci)
>>>>  	return 0;
>>>>  }
>>>>
>>>> -static int i3c_hci_runtime_suspend(struct device *dev)
>>>> +int i3c_hci_runtime_suspend(struct device *dev)
>>>>  {
>>>>  	struct i3c_hci *hci = dev_get_drvdata(dev);
>>>>  	int ret;
>>>> @@ -746,8 +746,9 @@ static int i3c_hci_runtime_suspend(struct device *dev)
>>>>
>>>>  	return 0;
>>>>  }
>>>> +EXPORT_SYMBOL_GPL(i3c_hci_runtime_suspend);
>>>>
>>>> -static int i3c_hci_runtime_resume(struct device *dev)
>>>> +int i3c_hci_runtime_resume(struct device *dev)
>>>>  {
>>>>  	struct i3c_hci *hci = dev_get_drvdata(dev);
>>>>  	int ret;
>>>> @@ -768,6 +769,7 @@ static int i3c_hci_runtime_resume(struct device *dev)
>>>>
>>>>  	return 0;
>>>>  }
>>>> +EXPORT_SYMBOL_GPL(i3c_hci_runtime_resume);
>>>>
>>>>  static int i3c_hci_suspend(struct device *dev)
>>>>  {
>>>> @@ -784,12 +786,14 @@ static int i3c_hci_resume_common(struct device *dev, bool rstdaa)
>>>>  	struct i3c_hci *hci = dev_get_drvdata(dev);
>>>>  	int ret;
>>>>
>>>> -	if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
>>>> -		return 0;
>>>> +	if (!(hci->quirks & HCI_QUIRK_RPM_PARENT_MANAGED)) {
>>>> +		if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
>>>> +			return 0;
>>>>
>>>> -	ret = pm_runtime_force_resume(dev);
>>>> -	if (ret)
>>>> -		return ret;
>>>> +		ret = pm_runtime_force_resume(dev);
>>>> +		if (ret)
>>>> +			return ret;
>>>> +	}
>>>>
>>>>  	ret = i3c_master_do_daa_ext(&hci->master, rstdaa);
>>>>  	if (ret)
>>>> @@ -812,8 +816,6 @@ static int i3c_hci_restore(struct device *dev)
>>>>  	return i3c_hci_resume_common(dev, true);
>>>>  }
>>>>
>>>> -#define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
>>>> -
>>>>  static void i3c_hci_rpm_enable(struct device *dev)
>>>>  {
>>>>  	struct i3c_hci *hci = dev_get_drvdata(dev);
>>>> @@ -962,6 +964,11 @@ static int i3c_hci_probe(struct platform_device *pdev)
>>>>  	if (hci->quirks & HCI_QUIRK_RPM_IBI_ALLOWED)
>>>>  		hci->master.rpm_ibi_allowed = true;
>>>>
>>>> +	if (hci->quirks & HCI_QUIRK_RPM_PARENT_MANAGED) {
>>>> +		hci->master.rpm_dev = pdev->dev.parent;
>>>> +		hci->master.rpm_allowed = true;
>>>> +	}
>>>> +
>>>>  	return i3c_master_register(&hci->master, &pdev->dev, &i3c_hci_ops, false);
>>>>  }
>>>>
>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
>>>> index 819328a85b84..d0e7ad58ac15 100644
>>>> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
>>>> @@ -147,6 +147,7 @@ struct i3c_hci_dev_data {
>>>>  #define HCI_QUIRK_RESP_BUF_THLD		BIT(4)  /* Set resp buf thld to 0 for AMD platforms */
>>>>  #define HCI_QUIRK_RPM_ALLOWED		BIT(5)  /* Runtime PM allowed */
>>>>  #define HCI_QUIRK_RPM_IBI_ALLOWED	BIT(6)  /* IBI and Hot-Join allowed while runtime suspended */
>>>> +#define HCI_QUIRK_RPM_PARENT_MANAGED	BIT(7)  /* Runtime PM managed by parent device */
>>>>
>>>>  /* global functions */
>>>>  void mipi_i3c_hci_resume(struct i3c_hci *hci);
>>>> @@ -156,4 +157,9 @@ void amd_set_od_pp_timing(struct i3c_hci *hci);
>>>>  void amd_set_resp_buf_thld(struct i3c_hci *hci);
>>>>  void i3c_hci_sync_irq_inactive(struct i3c_hci *hci);
>>>>
>>>> +#define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
>>>> +
>>>> +int i3c_hci_runtime_suspend(struct device *dev);
>>>> +int i3c_hci_runtime_resume(struct device *dev);
>>>> +
>>>>  #endif
>>>> --
>>>> 2.51.0
>>>>
>>

Powered by blists - more mailing lists