i3c: mipi-i3c-hci-pci: Enable IBI while runtime suspended for Intel controllers

Set d3hot_delay to 0 for Intel controllers because a delay is not needed.

Signed-off-by Adrian Hunter <[email protected]>

---

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 1 +
 1 file changed, 1 insertion(+)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +1 -1

index 0f05a15c14c7..bc83caad4197 100644
@@ -164,6 +164,7 @@ static int intel_i3c_init(struct mipi_i3c_hci_pci *hci)
 	dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));
 
 	hci->pci->d3cold_delay = 0;
+	hci->pci->d3hot_delay = 0;
 
 	hci->private = host;
 	host->priv = priv;
--

2.51.0

When an IBI can be received after the controller is pm_runtime_put_autosuspend()'ed, the interrupt may occur just before the device is auto‑suspended. In such cases, the runtime PM core may not see any recent activity and may suspend the device earlier than intended.

Mark the controller as last busy whenever an IBI is queued (when rpm_ibi_allowed is set) so that the auto-suspend delay correctly reflects recent bus activity and avoids premature suspension.

Signed-off-by Adrian Hunter <[email protected]>

---
 drivers/i3c/master.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/i3c/master.c +5 -1

index bcc493dc9d04..dcc07ebc50a2 100644
@@ -2721,9 +2721,14 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
  */
 void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
 {
+	struct i3c_master_controller *master = i3c_dev_get_master(dev);
+
 	if (!dev->ibi || !slot)
 		return;
 
+	if (master->rpm_ibi_allowed)
+		pm_runtime_mark_last_busy(master->rpm_dev);
+
 	atomic_inc(&dev->ibi->pending_ibis);
 	queue_work(dev->ibi->wq, &slot->work);
 }
--

2.51.0

Quoted 5 lines · Adrian Hunter

When an IBI can be received after the controller is pm_runtime_put_autosuspend()'ed, the interrupt may occur just before the device is auto‑suspended. In such cases, the runtime PM core may not see any recent activity and may suspend the device earlier than intended.

Quoted 3 lines

Mark the controller as last busy whenever an IBI is queued (when rpm_ibi_allowed is set) so that the auto-suspend delay correctly reflects recent bus activity and avoids premature suspension.

look like this can't resolve problem. pm_runtime_mark_last_busy() just change dev->power.last_busy. If suspend before it, nothing happen.

irq use thread irq, in irq thread call pm_runtime_resume() if needs.

And this function call by irq handle, just put to work queue, what's impact if do nothing here?

Frank

Quoted 28 lines

Signed-off-by Adrian Hunter <[email protected]>

---
 drivers/i3c/master.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/i3c/master.c +5 -1

index bcc493dc9d04..dcc07ebc50a2 100644
@@ -2721,9 +2721,14 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
  */
 void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
 {
+	struct i3c_master_controller *master = i3c_dev_get_master(dev);
+
 	if (!dev->ibi || !slot)
 		return;
 
+	if (master->rpm_ibi_allowed)
+		pm_runtime_mark_last_busy(master->rpm_dev);
+
 	atomic_inc(&dev->ibi->pending_ibis);
 	queue_work(dev->ibi->wq, &slot->work);
 }
--

2.51.0

Quoted 6 lines · Frank Li

Quoted 5 lines · Adrian Hunter

When an IBI can be received after the controller is pm_runtime_put_autosuspend()'ed, the interrupt may occur just before the device is auto‑suspended. In such cases, the runtime PM core may not see any recent activity and may suspend the device earlier than intended.

Quoted 6 lines

Quoted 3 lines

Mark the controller as last busy whenever an IBI is queued (when rpm_ibi_allowed is set) so that the auto-suspend delay correctly reflects recent bus activity and avoids premature suspension.

look like this can't resolve problem. pm_runtime_mark_last_busy() just change dev->power.last_busy. If suspend before it, nothing happen.

It should be effective.

rpm_suspend() recalculates the autosuspend expiry time based on last_busy (see pm_runtime_autosuspend_expiration()) and restarts the timer is it is in the future.

Quoted 1 line

Quoted 4 lines

irq use thread irq, in irq thread call pm_runtime_resume() if needs.

And this function call by irq handle, just put to work queue, what's impact if do nothing here?

Just premature runtime suspension inconsistent with autosuspend_delay.

Quoted 32 lines

Frank

Quoted 28 lines

Signed-off-by Adrian Hunter <[email protected]>

---
 drivers/i3c/master.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/i3c/master.c +5 -1

index bcc493dc9d04..dcc07ebc50a2 100644
@@ -2721,9 +2721,14 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
  */
 void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
 {
+	struct i3c_master_controller *master = i3c_dev_get_master(dev);
+
 	if (!dev->ibi || !slot)
 		return;
 
+	if (master->rpm_ibi_allowed)
+		pm_runtime_mark_last_busy(master->rpm_dev);
+
 	atomic_inc(&dev->ibi->pending_ibis);
 	queue_work(dev->ibi->wq, &slot->work);
 }
--

2.51.0

Quoted 26 lines · Adrian Hunter

Quoted 6 lines · Frank Li

Quoted 5 lines · Adrian Hunter

When an IBI can be received after the controller is pm_runtime_put_autosuspend()'ed, the interrupt may occur just before the device is auto‑suspended. In such cases, the runtime PM core may not see any recent activity and may suspend the device earlier than intended.

Quoted 6 lines

Quoted 3 lines

Mark the controller as last busy whenever an IBI is queued (when rpm_ibi_allowed is set) so that the auto-suspend delay correctly reflects recent bus activity and avoids premature suspension.

look like this can't resolve problem. pm_runtime_mark_last_busy() just change dev->power.last_busy. If suspend before it, nothing happen.

It should be effective.

rpm_suspend() recalculates the autosuspend expiry time based on last_busy (see pm_runtime_autosuspend_expiration()) and restarts the timer is it is in the future.

Quoted 5 lines

irq use thread irq, in irq thread call pm_runtime_resume() if needs.

And this function call by irq handle, just put to work queue, what's impact if do nothing here?

Quoted 1 line

Just premature runtime suspension inconsistent with autosuspend_delay.

CPU 0 CPU 1
1. rpm_suspend() 2. pm_runtime_mark_last_busy(master->rpm_dev)

if 2 happen before 1, it can extend suspend. 2 happen after 1, it should do nothing.

Frank

Quoted 33 lines

Quoted 32 lines

Frank

Quoted 28 lines

Signed-off-by Adrian Hunter <[email protected]>

---
 drivers/i3c/master.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/i3c/master.c +5 -1

index bcc493dc9d04..dcc07ebc50a2 100644
@@ -2721,9 +2721,14 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
  */
 void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
 {
+	struct i3c_master_controller *master = i3c_dev_get_master(dev);
+
 	if (!dev->ibi || !slot)
 		return;
 
+	if (master->rpm_ibi_allowed)
+		pm_runtime_mark_last_busy(master->rpm_dev);
+
 	atomic_inc(&dev->ibi->pending_ibis);
 	queue_work(dev->ibi->wq, &slot->work);
 }
--

2.51.0

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,

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 <[email protected]>

---
 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(-)

drivers/i3c/master/mipi-i3c-hci/core.c +16 -9

index ec4dbe64c35e..cb974b0f9e17 100644
@@ -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);
 }
 

drivers/i3c/master/mipi-i3c-hci/hci.h +6 -1

index 819328a85b84..d0e7ad58ac15 100644
@@ -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

Quoted 7 lines · Adrian Hunter

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.

Quoted 2 lines

For example, if the parent enables IBI-wakeup when transitioning into a low-power state,

Does your hardware support recieve IBI when runtime suspend?

Frank

Quoted 128 lines

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 <[email protected]>

---
 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(-)

drivers/i3c/master/mipi-i3c-hci/core.c +16 -9

index ec4dbe64c35e..cb974b0f9e17 100644
@@ -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);
 }
 

drivers/i3c/master/mipi-i3c-hci/hci.h +6 -1

index 819328a85b84..d0e7ad58ac15 100644
@@ -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

Quoted 11 lines · Frank Li

Quoted 9 lines · Adrian Hunter

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,

Quoted 1 line

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.

Quoted 132 lines

Frank

Quoted 128 lines

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 <[email protected]>

---
 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(-)

drivers/i3c/master/mipi-i3c-hci/core.c +16 -9

index ec4dbe64c35e..cb974b0f9e17 100644
@@ -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);
 }
 

drivers/i3c/master/mipi-i3c-hci/hci.h +6 -1

index 819328a85b84..d0e7ad58ac15 100644
@@ -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

Quoted 12 lines · Adrian Hunter

Quoted 11 lines · Frank Li

Quoted 9 lines · Adrian Hunter

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,

Quoted 6 lines

Quoted 1 line

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?

Frank

Quoted 133 lines

Quoted 132 lines

Frank

Quoted 128 lines

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 <[email protected]>

---
 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(-)

drivers/i3c/master/mipi-i3c-hci/core.c +16 -9

index ec4dbe64c35e..cb974b0f9e17 100644
@@ -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);
 }
 

drivers/i3c/master/mipi-i3c-hci/hci.h +6 -1

index 819328a85b84..d0e7ad58ac15 100644
@@ -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

Quoted 22 lines · Frank Li

Quoted 12 lines · Adrian Hunter

Quoted 11 lines · Frank Li

Quoted 9 lines · Adrian Hunter

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,

Quoted 6 lines

Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 136 lines

Frank

Quoted 133 lines

Quoted 132 lines

Frank

Quoted 128 lines

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 <[email protected]>

---
 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(-)

drivers/i3c/master/mipi-i3c-hci/core.c +16 -9

index ec4dbe64c35e..cb974b0f9e17 100644
@@ -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);
 }
 

drivers/i3c/master/mipi-i3c-hci/hci.h +6 -1

index 819328a85b84..d0e7ad58ac15 100644
@@ -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

Quoted 43 lines · Adrian Hunter

Quoted 22 lines · Frank Li

Quoted 12 lines · Adrian Hunter

Quoted 11 lines · Frank Li

Quoted 9 lines · Adrian Hunter

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,

Quoted 6 lines

Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 5 lines

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.

One instance 1 suspend, instance 2 running, PME is inactive, what's happen if instance 1 request IBI?

IBI will be missed?

Quoted 5 lines

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.

Does PME active auto by hardware or need software config?

Frank

Quoted 141 lines

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.

Quoted 136 lines

Frank

Quoted 133 lines

Quoted 132 lines

Frank

Quoted 128 lines

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 <[email protected]>

---
 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(-)

drivers/i3c/master/mipi-i3c-hci/core.c +16 -9

index ec4dbe64c35e..cb974b0f9e17 100644
@@ -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);
 }
 

drivers/i3c/master/mipi-i3c-hci/hci.h +6 -1

index 819328a85b84..d0e7ad58ac15 100644
@@ -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

Quoted 44 lines · Frank Li

Quoted 43 lines · Adrian Hunter

Quoted 22 lines · Frank Li

Quoted 12 lines · Adrian Hunter

Quoted 11 lines · Frank Li

Quoted 9 lines · Adrian Hunter

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,

Quoted 6 lines

Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 8 lines

Quoted 5 lines

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.

One instance 1 suspend, instance 2 running, PME is inactive, what's happen if instance 1 request IBI?

Nothing will happen. Instance 1 I3C bus is not operational and there can be no PME when the PCI device is not in a low power state (D3hot)

Quoted 2 lines

IBI will be missed?

Possibly not if instance 1 is eventually resumed and the I3C device requesting the IBI has not yet given up.

Quoted 6 lines

Quoted 5 lines

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.

Quoted 1 line

Does PME active auto by hardware or need software config?

PCI devices (hardware) advertise their PME capability in terms of which states are capable of PMEs. Currently the Intel LPSS I3C device lists only D3hot.

The PCI subsystem (software) automatically enables the PME before runtime suspend if the target power state allows it.

Quoted 144 lines

Frank

Quoted 141 lines

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.

Quoted 136 lines

Frank

Quoted 133 lines

Quoted 132 lines

Frank

Quoted 128 lines

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 <[email protected]>

---
 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(-)

drivers/i3c/master/mipi-i3c-hci/core.c +16 -9

index ec4dbe64c35e..cb974b0f9e17 100644
@@ -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);
 }
 

drivers/i3c/master/mipi-i3c-hci/hci.h +6 -1

index 819328a85b84..d0e7ad58ac15 100644
@@ -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

Quoted 44 lines · Frank Li

Quoted 43 lines · Adrian Hunter

Quoted 22 lines · Frank Li

Quoted 12 lines · Adrian Hunter

Quoted 11 lines · Frank Li

Quoted 9 lines · Adrian Hunter

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,

Quoted 6 lines

Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 8 lines

Quoted 5 lines

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.

One instance 1 suspend, instance 2 running, PME is inactive, what's happen if instance 1 request IBI?

Nothing will happen. Instance 1 I3C bus is not operational and there can be no PME when the PCI device is not in a low power state (D3hot)

Quoted 2 lines

IBI will be missed?

Possibly not if instance 1 is eventually resumed and the I3C device requesting the IBI has not yet given up.

Quoted 6 lines

Quoted 5 lines

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.

Quoted 1 line

Does PME active auto by hardware or need software config?

PCI devices (hardware) advertise their PME capability in terms of which states are capable of PMEs. Currently the Intel LPSS I3C device lists only D3hot.

The PCI subsystem (software) automatically enables the PME before runtime suspend if the target power state allows it.

Quoted 144 lines

Frank

Quoted 141 lines

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.

Quoted 136 lines

Frank

Quoted 133 lines

Quoted 132 lines

Frank

Quoted 128 lines

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 <[email protected]>

---
 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(-)

drivers/i3c/master/mipi-i3c-hci/core.c +16 -9

index ec4dbe64c35e..cb974b0f9e17 100644
@@ -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);
 }
 

drivers/i3c/master/mipi-i3c-hci/hci.h +6 -1

index 819328a85b84..d0e7ad58ac15 100644
@@ -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

Does your device Hierarchy look like

           PCI device
               |
       -----------------
      HCI1            HCI2
       |               |
     I3C M1          I3C M2

You want HCI1 and HCI2 suspened only when both HCI1 and HCI2 can enter RM time suspend status?

Device Link can link two devices, but not sure if it can handle cyclic case, HCI1 and HCI2 depend each other.

Or you create common power domain for HCI1 and HCI2, in power domain to handle suspend.

It'd better ask run time pm owner to provide better suggestion.

+ Rafael

Rafael, this is not at all urgent, but when you have time please have a look at this patch set. There is also some more explanation below. Is it acceptable? Is there a better way?

Quoted 9 lines · Frank Li

Does your device Hierarchy look like

           PCI device
               |
       -----------------
      HCI1            HCI2
       |               |
     I3C M1          I3C M2

Yes and no. There is only 1 real device : the PCI device. It implements the MIPI I3C HCI standard which allows multiple I3C bus controllers in one device (in our case 2 controllers).

The PCI driver mipi-i3c-hci-pci creates 2 platform devices, one for each controller. The platform driver is mipi-i3c-hci:

Driver Device Bus

mipi-i3c-hci-pci 0000:00:11.1 PCI

                            /            \

mipi-i3c-hci intel-lpss-i3c.0 intel-lpss-i3c.1 Platform

LPSS I3C also supports wake-up from in-band interrupt (IBI) via PCI PME. The PME only works when the PCI device is in a low power state (D3hot in our case). Also the PME is effectively shared by the 2 controllers i.e. an IBI signal (the controller's SDA line pulled low) for either controller will cause the PME.

That means there are only 2 valid configurations:

1: Both controllers enabled to receive IBIs
PCI device D0
Controller 1 Enabled
Controller 2 Enabled

2: Both controllers disabled to enable PME wakeup
PCI device D3hot
Controller 1 Disabled
Controller 2 Disabled

However, represented as platform devices, the 2 controllers runtime suspend and resume independently from each other. Whereas they effectively need to runtime suspend (or resume) at the same time.

The proposed solution is for the PCI driver mipi-i3c-hci-pci to take over managing runtime PM for both controllers, calling into mipi-i3c-hci when it is safe to do so, to save/restore state and disable/enable the controllers one after the other.

Current situation (I3C next branch):

PCI device Runtime PM enabled, dependent on child devices
PCI subsystem controls PCI device power state

Controller 1 Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: disable and save state Runtime resume: restore state and enable

Controller 2 Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: disable and save state Runtime resume: restore state and enable

Proposed (this patch set):

PCI device Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the PCI device For each controller:
Call into mipi-i3c-hci (when it is safe)
Runtime suspend: disable and save state Runtime resume: restore state and enable

Controller 1 Runtime PM disabled

Controller 2 Runtime PM disabled

Quoted 6 lines

You want HCI1 and HCI2 suspened only when both HCI1 and HCI2 can enter RM time suspend status?

Quoted 7 lines

Device Link can link two devices, but not sure if it can handle cyclic case, HCI1 and HCI2 depend each other.

Or you create common power domain for HCI1 and HCI2, in power domain to handle suspend.

It'd better ask run time pm owner to provide better suggestion.

Ok

Quoted 81 lines · Adrian Hunter

+ Rafael

Rafael, this is not at all urgent, but when you have time please have a look at this patch set. There is also some more explanation below. Is it acceptable? Is there a better way?

Quoted 8 lines · Frank Li

Does your device Hierarchy look like

            PCI device
                |
        -----------------
       HCI1            HCI2
        |               |
      I3C M1          I3C M2

Yes and no. There is only 1 real device : the PCI device. It implements the MIPI I3C HCI standard which allows multiple I3C bus controllers in one device (in our case 2 controllers).

The PCI driver mipi-i3c-hci-pci creates 2 platform devices, one for each controller. The platform driver is mipi-i3c-hci:

Driver Device Bus

  mipi-i3c-hci-pci            0000:00:11.1                 PCI
                             /            \
  mipi-i3c-hci      intel-lpss-i3c.0  intel-lpss-i3c.1     Platform

LPSS I3C also supports wake-up from in-band interrupt (IBI) via PCI PME. The PME only works when the PCI device is in a low power state (D3hot in our case). Also the PME is effectively shared by the 2 controllers i.e. an IBI signal (the controller's SDA line pulled low) for either controller will cause the PME.

That means there are only 2 valid configurations:

1: Both controllers enabled to receive IBIs
PCI device D0
Controller 1 Enabled
Controller 2 Enabled

2: Both controllers disabled to enable PME wakeup
PCI device D3hot
Controller 1 Disabled
Controller 2 Disabled

However, represented as platform devices, the 2 controllers runtime suspend and resume independently from each other. Whereas they effectively need to runtime suspend (or resume) at the same time.

The proposed solution is for the PCI driver mipi-i3c-hci-pci to take over managing runtime PM for both controllers, calling into mipi-i3c-hci when it is safe to do so, to save/restore state and disable/enable the controllers one after the other.

Current situation (I3C next branch):

PCI device Runtime PM enabled, dependent on child devices
PCI subsystem controls PCI device power state

Controller 1 Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: disable and save state Runtime resume: restore state and enable

Controller 2 Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: disable and save state Runtime resume: restore state and enable

Proposed (this patch set):

PCI device Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the PCI device For each controller:
Call into mipi-i3c-hci (when it is safe)
Runtime suspend: disable and save state Runtime resume: restore state and enable

Quoted 3 lines

Controller 1 Runtime PM disabled

Controller 2 Runtime PM disabled

Controller 1/2 is child device of PCI device.

So first patch "i3c: master: Allow controller drivers to select runtime PM device" is not necessary.

You can enable controller 1 and 2 Runtime PM with dummy operation.

When controller 1 run time suspend, parent PCI device will reduce ref counter when controller 2 run time suspend, parent PCI device will reduce ref counter

Only runtime pm reference counter of PCI device is 0, PCI device's run time suspend will be called, you can do actual disable and save work.

Quoted 92 lines · Frank Li

Quoted 81 lines · Adrian Hunter

+ Rafael

Rafael, this is not at all urgent, but when you have time please have a look at this patch set.  There is also some more explanation below.  Is it acceptable?  Is there a better way?

Quoted 8 lines · Frank Li

Does your device Hierarchy look like

            PCI device
                |
        -----------------
       HCI1            HCI2
        |               |
      I3C M1          I3C M2

Yes and no.  There is only 1 real device : the PCI device. It implements the MIPI I3C HCI standard which allows multiple I3C bus controllers in one device (in our case 2 controllers).

The PCI driver mipi-i3c-hci-pci creates 2 platform devices, one for each controller.  The platform driver is mipi-i3c-hci:

  Driver                        Device                     Bus

  mipi-i3c-hci-pci            0000:00:11.1                 PCI /            \
  mipi-i3c-hci      intel-lpss-i3c.0  intel-lpss-i3c.1     Platform

LPSS I3C also supports wake-up from in-band interrupt (IBI) via PCI PME.  The PME only works when the PCI device is in a low power state (D3hot in our case).  Also the PME is effectively shared by the 2 controllers i.e. an IBI signal (the controller's SDA line pulled low) for either controller will cause the PME.

That means there are only 2 valid configurations:

    1: Both controllers enabled to receive IBIs
        PCI device    D0
        Controller 1    Enabled
        Controller 2    Enabled

    2: Both controllers disabled to enable PME wakeup PCI device    D3hot
        Controller 1    Disabled
        Controller 2    Disabled

However, represented as platform devices, the 2 controllers runtime suspend and resume independently from each other. Whereas they effectively need to runtime suspend (or resume) at the same time.

The proposed solution is for the PCI driver mipi-i3c-hci-pci to take over managing runtime PM for both controllers, calling into mipi-i3c-hci when it is safe to do so, to save/restore state and disable/enable the controllers one after the other.

Current situation (I3C next branch):

    PCI device    Runtime PM enabled, dependent on child devices PCI subsystem controls PCI device power state

    Controller 1    Runtime PM enabled plus auto-suspend I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: disable and save state Runtime resume: restore state and enable

    Controller 2    Runtime PM enabled plus auto-suspend I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: disable and save state Runtime resume: restore state and enable

Proposed (this patch set):

    PCI device    Runtime PM enabled plus auto-suspend I3C subsystem runtime PM gets/puts the PCI device For each controller:
                Call into mipi-i3c-hci (when it is safe) Runtime suspend: disable and save state Runtime resume: restore state and enable

Quoted 3 lines

    Controller 1    Runtime PM disabled

    Controller 2    Runtime PM disabled

Controller 1/2 is child device of PCI device.

So first patch "i3c: master: Allow controller drivers to select runtime PM device" is not necessary.

You can enable controller 1 and 2 Runtime PM with dummy operation.

Quoted 5 lines

When controller 1 run time suspend, parent PCI device will reduce ref counter when controller 2 run time suspend, parent PCI device will reduce ref counter

Only runtime pm reference counter of PCI device is 0, PCI device's run time suspend will be called, you can do actual disable and save work.

This sounds to me like it should work.

Quoted 94 lines · Rafael J

Quoted 92 lines · Frank Li

Quoted 81 lines · Adrian Hunter

+ Rafael

Rafael, this is not at all urgent, but when you have time please have a look at this patch set. There is also some more explanation below. Is it acceptable? Is there a better way?

Quoted 8 lines · Frank Li

Does your device Hierarchy look like

            PCI device
                |
        -----------------
       HCI1            HCI2
        |               |
      I3C M1          I3C M2

Yes and no. There is only 1 real device : the PCI device. It implements the MIPI I3C HCI standard which allows multiple I3C bus controllers in one device (in our case 2 controllers).

The PCI driver mipi-i3c-hci-pci creates 2 platform devices, one for each controller. The platform driver is mipi-i3c-hci:

Driver Device Bus

  mipi-i3c-hci-pci            0000:00:11.1                 PCI
                             /            \
  mipi-i3c-hci      intel-lpss-i3c.0  intel-lpss-i3c.1     Platform

LPSS I3C also supports wake-up from in-band interrupt (IBI) via PCI PME. The PME only works when the PCI device is in a low power state (D3hot in our case). Also the PME is effectively shared by the 2 controllers i.e. an IBI signal (the controller's SDA line pulled low) for either controller will cause the PME.

That means there are only 2 valid configurations:

1: Both controllers enabled to receive IBIs
PCI device D0
Controller 1 Enabled
Controller 2 Enabled

2: Both controllers disabled to enable PME wakeup
PCI device D3hot
Controller 1 Disabled
Controller 2 Disabled

However, represented as platform devices, the 2 controllers runtime suspend and resume independently from each other. Whereas they effectively need to runtime suspend (or resume) at the same time.

The proposed solution is for the PCI driver mipi-i3c-hci-pci to take over managing runtime PM for both controllers, calling into mipi-i3c-hci when it is safe to do so, to save/restore state and disable/enable the controllers one after the other.

Current situation (I3C next branch):

PCI device Runtime PM enabled, dependent on child devices
PCI subsystem controls PCI device power state

Controller 1 Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: disable and save state Runtime resume: restore state and enable

Controller 2 Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: disable and save state Runtime resume: restore state and enable

Proposed (this patch set):

PCI device Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the PCI device For each controller:
Call into mipi-i3c-hci (when it is safe)
Runtime suspend: disable and save state Runtime resume: restore state and enable

Quoted 3 lines

Controller 1 Runtime PM disabled

Controller 2 Runtime PM disabled

Controller 1/2 is child device of PCI device.

So first patch "i3c: master: Allow controller drivers to select runtime PM device" is not necessary.

You can enable controller 1 and 2 Runtime PM with dummy operation.

Quoted 7 lines

Quoted 5 lines

When controller 1 run time suspend, parent PCI device will reduce ref counter when controller 2 run time suspend, parent PCI device will reduce ref counter

Only runtime pm reference counter of PCI device is 0, PCI device's run time suspend will be called, you can do actual disable and save work.

This sounds to me like it should work.

So, like this, then?

PCI device Runtime PM enabled
For each controller:
Call into mipi-i3c-hci (when it is safe)
Runtime suspend: disable and save state Runtime resume: restore state and enable

Controller 1 Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: does nothing
Runtime resume: does nothing

Controller 2 Runtime PM enabled plus auto-suspend
I3C subsystem runtime PM gets/puts the Platform device Runtime suspend: does nothing
Runtime resume: does nothing