i3c: mipi-i3c-hci-pci: Enable IBI while runtime suspended for Intel controllers
v2 cover i3c: mipi-i3c-hci-pci: Enable IBI while runtime suspended for Intel controllers
Hi
Changes in V2:
i3c: mipi-i3c-hci-pci: Set d3hot_delay to 0 for Intel controllers
Add Frank's Rev'd-by
i3c: master: Allow controller drivers to select runtime PM device
Patch dropped
i3c: master: Mark last_busy on IBI when runtime PM is allowed
Adjusted slightly for earlier changes
i3c: mipi-i3c-hci: Allow parent to manage runtime PM
For HCI_QUIRK_RPM_PARENT_MANAGED case, change from disabling runtime PM to instead causing the runtime PM callbacks to do nothing
i3c: mipi-i3c-hci-pci: Add optional ability to manage child runtime PM
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.
i3c: mipi-i3c-hci-pci: Enable IBI while runtime suspended for Intel controllers
Retain HCI_QUIRK_RPM_ALLOWED
Amend commit message accordingly
Here are patches related to enabling IBI while runtime suspended for Intel controllers.
Intel LPSS I3C controllers can wake from runtime suspend to receive in-band interrupts (IBIs).
It is non-trivial to implement because the parent PCI device has 2 I3C bus instances (MIPI I3C HCI Multi-Bus Instance capability) represented by platform devices with a separate driver, but the IBI-wakeup is shared by both, which means runtime PM has to be managed by the parent PCI driver.
To make that work, the PCI driver handles runtime PM, but leverages the mipi-i3c-hci platform driver's functionality for saving and restoring controller state.
Adrian Hunter (6):
i3c: mipi-i3c-hci-pci: Set d3hot_delay to 0 for Intel controllers
i3c: master: Mark last_busy on IBI when runtime PM is allowed
i3c: mipi-i3c-hci: Add quirk to allow IBI while runtime suspended
i3c: mipi-i3c-hci: Allow parent to manage runtime PM
i3c: mipi-i3c-hci-pci: Add optional ability to manage child runtime PM
i3c: mipi-i3c-hci-pci: Enable IBI while runtime suspended for Intel controllers
drivers/i3c/master.c | 5 + drivers/i3c/master/mipi-i3c-hci/core.c | 35 +++++- drivers/i3c/master/mipi-i3c-hci/hci.h | 7 ++ drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 138 +++++++++++++++++++++ 4 files changed, 180 insertions(+), 5 deletions(-)
Regards
Adrian
PATCH v2 1/6 i3c: mipi-i3c-hci-pci: Set d3hot_delay to 0 for Intel controllers
Set d3hot_delay to 0 for Intel controllers because a delay is not needed.
---
Changes in V2:
Add Frank's Rev'd-by
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 -0
@@ -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;
Signature · 1 line
2.51.0
PATCH v2 2/6 i3c: master: Mark last_busy on IBI when runtime PM is allowed
In reply to Adrian HunterWhen 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.
---
Changes in V2:
Adjusted slightly for earlier changes
drivers/i3c/master.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/i3c/master.c +5 -0
@@ -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->dev.parent); +atomic_inc (&dev->ibi->pending_ibis);queue_work (dev->ibi->wq, &slot->work); }
Signature · 1 line
2.51.0
Frank Li
reviewerRe: [PATCH V2 2/6] i3c: master: Mark last_busy on IBI when runtime PM is allowed
Quoted 11 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.
Mark the controller as last busy whenever an IBI is queued (when
rpm_ibi_allowedis set) so that the auto-suspend delay correctly reflects recent bus activity and avoids premature suspension.Signed-off-by Adrian Hunter <[email protected]>---
Although it is no harmful, I think it is not necessary to mark last busy.
schedule's workqueue task to do i3c transfer, which will call run time resume.
Are sure it will block your function without this patch?
Frank
Quoted 32 lines
Changes in V2:
Adjusted slightly for earlier changes
drivers/i3c/master.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/i3c/master.c +5
-1@@ -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->dev.parent); +atomic_inc (&dev->ibi->pending_ibis);queue_work (dev->ibi->wq, &slot->work); } --2.51.0
Re: [PATCH V2 2/6] i3c: master: Mark last_busy on IBI when runtime PM is allowed
Quoted 13 lines · Frank Li
Quoted 11 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.
Mark the controller as last busy whenever an IBI is queued (when
rpm_ibi_allowedis set) so that the auto-suspend delay correctly reflects recent bus activity and avoids premature suspension.Signed-off-by Adrian Hunter <[email protected]>---
Quoted 6 lines
Although it is no harmful, I think it is not necessary to mark last busy.
schedule's workqueue task to do i3c transfer, which will call run time resume.
Are sure it will block your function without this patch?
It is not necessary at this time. I wanted to cover the case where an IBI is not followed by a transfer from the target device driver. Perhaps a device could operate almost entirely using IBIs, but there is nothing like that presently.
Let me know if you think we should leave it for now.
Quoted 35 lines
Frank
Quoted 32 lines
Changes in V2:
Adjusted slightly for earlier changes
drivers/i3c/master.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/i3c/master.c +5
-1@@ -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->dev.parent); +atomic_inc (&dev->ibi->pending_ibis);queue_work (dev->ibi->wq, &slot->work); } --2.51.0
Frank Li
reviewerRe: [PATCH V2 2/6] i3c: master: Mark last_busy on IBI when runtime PM is allowed
Quoted 16 lines · Adrian Hunter
Quoted 15 lines · Frank Li
Quoted 11 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.
Mark the controller as last busy whenever an IBI is queued (when
rpm_ibi_allowedis set) so that the auto-suspend delay correctly reflects recent bus activity and avoids premature suspension.Signed-off-by Adrian Hunter <[email protected]>---
Although it is no harmful, I think it is not necessary to mark last busy.
Quoted 8 lines
Quoted 4 lines
schedule's workqueue task to do i3c transfer, which will call run time resume.
Are sure it will block your function without this patch?
It is not necessary at this time. I wanted to cover the case where an IBI is not followed by a transfer from the target device driver.
look like it is impossible. Device raise IBI, which means device need do some things. otherwise, why raise IBI.
Let's defer this special case when we really meet in future.
Frank
Quoted 46 lines
Perhaps a device could operate almost entirely using IBIs, but there is nothing like that presently.
Let me know if you think we should leave it for now.
Quoted 26 lines
Frank
Quoted 23 lines
Changes in V2:
Adjusted slightly for earlier changes
drivers/i3c/master.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/i3c/master.c +2
-0@@ -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 ;Quoted 9 lines
Quoted 9 lines
drivers/i3c/master.c +3
-1+if (master->rpm_ibi_allowed) +pm_runtime_mark_last_busy (master->dev.parent); +atomic_inc (&dev->ibi->pending_ibis);queue_work (dev->ibi->wq, &slot->work); } --2.51.0
--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
Re: [PATCH V2 2/6] i3c: master: Mark last_busy on IBI when runtime PM is allowed
Quoted 20 lines · Frank Li
Quoted 19 lines · Adrian Hunter
Quoted 18 lines · Frank Li
Quoted 11 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.
Mark the controller as last busy whenever an IBI is queued (when
rpm_ibi_allowedis set) so that the auto-suspend delay correctly reflects recent bus activity and avoids premature suspension.Signed-off-by Adrian Hunter <[email protected]>---
Although it is no harmful, I think it is not necessary to mark last busy.
schedule's workqueue task to do i3c transfer, which will call run time resume.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Are sure it will block your function without this patch?
It is not necessary at this time. I wanted to cover the case where an IBI is not followed by a transfer from the target device driver.
look like it is impossible. Device raise IBI, which means device need do some things. otherwise, why raise IBI.
An IBI has a payload. A device could provide all the data in the IBI.
Quoted 2 lines
Let's defer this special case when we really meet in future.
Sure
PATCH v2 3/6 i3c: mipi-i3c-hci: Add quirk to allow IBI while runtime suspended
In reply to Adrian HunterSome I3C controllers can be automatically runtime-resumed in order to handle in-band interrupts (IBIs), meaning that runtime suspend does not need to be blocked when IBIs are enabled.
For example, a PCI-attached controller in a low-power state may generate 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, allowing the IBI to be received without requiring the controller to remain active.
Introduce a new quirk, HCI_QUIRK_RPM_IBI_ALLOWED, so that drivers can opt-in to this capability via driver data.
---
Changes in V2:
None
drivers/i3c/master/mipi-i3c-hci/core.c | 3 +++ drivers/i3c/master/mipi-i3c-hci/hci.h | 1 + 2 files changed, 4 insertions(+)
drivers/i3c/master/mipi-i3c-hci/core.c +3 -0
@@ -959,6 +959,9 @@ static int i3c_hci_probe(struct platform_device *pdev)if (hci->quirks &HCI_QUIRK_RPM_ALLOWED )i3c_hci_rpm_enable (&pdev->dev); +if (hci->quirks &HCI_QUIRK_RPM_IBI_ALLOWED ) + hci->master.rpm_ibi_allowed = true; +return i3c_master_register (&hci->master, &pdev->dev, &i3c_hci_ops, false); }
drivers/i3c/master/mipi-i3c-hci/hci.h +1 -0
@@ -146,6 +146,7 @@ struct i3c_hci_dev_data {#define HCI_QUIRK_OD_PP_TIMING BIT(3)/* Set OD and PP timings for AMD platforms */ #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 */ /* global functions */ void mipi_i3c_hci_resume (struct i3c_hci *hci);
Signature · 1 line
2.51.0
Re: [PATCH V2 3/6] i3c: mipi-i3c-hci: Add quirk to allow IBI while runtime suspended
Quoted 10 lines · Adrian Hunter
Some I3C controllers can be automatically runtime-resumed in order to handle in-band interrupts (IBIs), meaning that runtime suspend does not need to be blocked when IBIs are enabled.
For example, a PCI-attached controller in a low-power state may generate 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, allowing the IBI to be received without requiring the controller to remain active.
Quoted 5 lines
Introduce a new quirk,
HCI_QUIRK_RPM_IBI_ALLOWED, so that drivers can opt-in to this capability via driver data.Signed-off-by Adrian Hunter <[email protected]>---
Quoted 40 lines
Changes in V2:
None
drivers/i3c/master/mipi-i3c-hci/core.c | 3 +++ drivers/i3c/master/mipi-i3c-hci/hci.h | 1 + 2 files changed, 4 insertions(+)drivers/i3c/master/mipi-i3c-hci/core.c +3
-0@@ -959,6 +959,9 @@ static int i3c_hci_probe(struct platform_device *pdev)if (hci->quirks &HCI_QUIRK_RPM_ALLOWED )i3c_hci_rpm_enable (&pdev->dev); +if (hci->quirks &HCI_QUIRK_RPM_IBI_ALLOWED ) + hci->master.rpm_ibi_allowed = true; +return i3c_master_register (&hci->master, &pdev->dev, &i3c_hci_ops, false); }drivers/i3c/master/mipi-i3c-hci/hci.h +1
-1@@ -146,6 +146,7 @@ struct i3c_hci_dev_data {#define HCI_QUIRK_OD_PP_TIMING BIT(3)/* Set OD and PP timings for AMD platforms */ #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 */ /* global functions */ void mipi_i3c_hci_resume (struct i3c_hci *hci); --2.51.0
PATCH v2 4/6 i3c: mipi-i3c-hci: Allow parent to manage runtime PM
In reply to Adrian HunterSome 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:
* Export the low-level runtime PM suspend and resume helpers so that the parent can explicitly invoke them.
* Add a new quirk, HCI_QUIRK_RPM_PARENT_MANAGED, allowing platforms to bypass per-instance runtime PM callbacks and delegate control to the parent device.
* 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.
---
Changes in V2:
For HCI_QUIRK_RPM_PARENT_MANAGED case, change from disabling runtime PM to instead causing the runtime PM callbacks to do nothing
drivers/i3c/master/mipi-i3c-hci/core.c | 28 ++++++++++++++++++++++---- drivers/i3c/master/mipi-i3c-hci/hci.h | 6 ++++++ 2 files changed, 30 insertions(+), 4 deletions(-)
drivers/i3c/master/mipi-i3c-hci/core.c +24 -4
@@ -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_rpm_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_rpm_suspend); -static int i3c_hci_runtime_resume (struct device *dev) +int i3c_hci_rpm_resume (struct device *dev) {struct i3c_hci *hci =dev_get_drvdata (dev);int ret;
@@ -768,6 +769,27 @@ static int i3c_hci_runtime_resume(struct device *dev)return 0 ; } +EXPORT_SYMBOL_GPL (i3c_hci_rpm_resume); + +static int i3c_hci_runtime_suspend (struct device *dev) +{ +struct i3c_hci *hci =dev_get_drvdata (dev); + +if (hci->quirks &HCI_QUIRK_RPM_PARENT_MANAGED ) +return 0 ; + +return i3c_hci_rpm_suspend (dev); +} + +static int i3c_hci_runtime_resume (struct device *dev) +{ +struct i3c_hci *hci =dev_get_drvdata (dev); + +if (hci->quirks &HCI_QUIRK_RPM_PARENT_MANAGED ) +return 0 ; + +return i3c_hci_rpm_resume (dev); +}static int i3c_hci_suspend (struct device *dev) {
@@ -812,8 +834,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);
drivers/i3c/master/mipi-i3c-hci/hci.h +6 -0
@@ -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_rpm_suspend (struct device *dev); +int i3c_hci_rpm_resume (struct device *dev); +#endif
Signature · 1 line
2.51.0
PATCH v2 5/6 i3c: mipi-i3c-hci-pci: Add optional ability to manage child runtime PM
In reply to Adrian HunterSome 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.
---
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 | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+)
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +134 -0
@@ -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,128 @@ 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) { +if (ret == -EAGAIN || ret == -EBUSY ) +pm_runtime_mark_last_busy (&hci->pci->dev); +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[] = {
Signature · 1 line
2.51.0
Frank Li
reviewerRe: [PATCH V2 5/6] i3c: mipi-i3c-hci-pci: Add optional ability to manage child runtime PM
Quoted 33 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 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 <[email protected]>---
...
Quoted 4 lines
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +4
-0+ +static int mipi_i3c_hci_pci_suspend (struct device *dev) +{ +struct mipi_i3c_hci_pci *hci =dev_get_drvdata (dev);
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static const struct mipi_i3c_hci_pci_info intel_si_2_info = { · line 320
Quoted 10 lines
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10
-0+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.
Frank
Quoted 39 lines
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +26
-1+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[] = { --2.51.0
Re: [PATCH V2 5/6] i3c: mipi-i3c-hci-pci: Add optional ability to manage child runtime PM
Quoted 36 lines · Frank Li
Quoted 33 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 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 <[email protected]>---
...
Quoted 16 lines
Quoted 4 lines
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +4
-0+ +static int mipi_i3c_hci_pci_suspend (struct device *dev) +{ +struct mipi_i3c_hci_pci *hci =dev_get_drvdata (dev);drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static const struct mipi_i3c_hci_pci_info intel_si_2_info = { · line 320
Quoted 10 lines
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10
-0+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?
Quoted 37 lines
Quoted 37 lines
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +26
-1+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[] = { --
Re: [PATCH V2 5/6] i3c: mipi-i3c-hci-pci: Add optional ability to manage child runtime PM
In reply to Frank LiQuoted 35 lines · Frank Li
Quoted 33 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 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 <[email protected]>---
...
Quoted 16 lines
Quoted 4 lines
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +4
-0+ +static int mipi_i3c_hci_pci_suspend (struct device *dev) +{ +struct mipi_i3c_hci_pci *hci =dev_get_drvdata (dev);drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static const struct mipi_i3c_hci_pci_info intel_si_2_info = { · line 320
Quoted 10 lines
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10
-0+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.
In face it is not necessary because the driver is no longer using auto-suspend. I will take it out.
PATCH v2 6/6 i3c: mipi-i3c-hci-pci: Enable IBI while runtime suspended for Intel controllers
In reply to Adrian HunterIntel LPSS I3C controllers can wake from runtime suspend to receive in-band interrupts (IBIs), and they also implement the MIPI I3C HCI Multi-Bus Instance capability. When multiple I3C bus instances share the same PCI wakeup, the PCI parent must coordinate runtime PM so that all instances suspend together and their mipi-i3c-hci runtime suspend callbacks are invoked in a consistent manner.
Enable IBI-based wakeup by setting HCI_QUIRK_RPM_IBI_ALLOWED for the intel-lpss-i3c platform device. Also set HCI_QUIRK_RPM_PARENT_MANAGED so that the mipi-i3c-hci core driver expects runtime PM to be controlled by the PCI parent rather than by individual instances. For all Intel HCI PCI configurations, enable the corresponding control_instance_pm flag in the PCI driver.
---
Changes in V2:
Retain HCI_QUIRK_RPM_ALLOWED
Amend commit message accordingly
drivers/i3c/master/mipi-i3c-hci/core.c | 4 +++- drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-)
drivers/i3c/master/mipi-i3c-hci/core.c +3 -1
@@ -1005,7 +1005,9 @@ static const struct acpi_device_id i3c_hci_acpi_match[] = {MODULE_DEVICE_TABLE (acpi, i3c_hci_acpi_match);static const struct platform_device_id i3c_hci_driver_ids[] = { - { .name ="intel-lpss-i3c" ,HCI_QUIRK_RPM_ALLOWED }, + { .name ="intel-lpss-i3c" ,HCI_QUIRK_RPM_ALLOWED | +HCI_QUIRK_RPM_IBI_ALLOWED | +HCI_QUIRK_RPM_PARENT_MANAGED }, {/* sentinel */ } };MODULE_DEVICE_TABLE (platform, i3c_hci_driver_ids);
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +3 -0
@@ -200,6 +200,7 @@ static const struct mipi_i3c_hci_pci_info intel_mi_1_info = { .id = {0, 1}, .instance_offset = {0, 0x400}, .instance_count = 2, + .control_instance_pm = true, }; static const struct mipi_i3c_hci_pci_info intel_mi_2_info = {
@@ -209,6 +210,7 @@ static const struct mipi_i3c_hci_pci_info intel_mi_2_info = { .id = {2, 3}, .instance_offset = {0, 0x400}, .instance_count = 2, + .control_instance_pm = true, }; static const struct mipi_i3c_hci_pci_info intel_si_2_info = {
@@ -218,6 +220,7 @@ static const struct mipi_i3c_hci_pci_info intel_si_2_info = { .id = {2}, .instance_offset = {0}, .instance_count = 1, + .control_instance_pm = true, };static int mipi_i3c_hci_pci_find_instance (struct mipi_i3c_hci_pci *hci,struct device *dev)
Signature · 1 line
2.51.0
Quoted 14 lines · Adrian Hunter
Intel LPSS I3C controllers can wake from runtime suspend to receive in-band interrupts (IBIs), and they also implement the MIPI I3C HCI Multi-Bus Instance capability. When multiple I3C bus instances share the same PCI wakeup, the PCI parent must coordinate runtime PM so that all instances suspend together and their mipi-i3c-hci runtime suspend callbacks are invoked in a consistent manner.
Enable IBI-based wakeup by setting
HCI_QUIRK_RPM_IBI_ALLOWEDfor the intel-lpss-i3c platform device. Also setHCI_QUIRK_RPM_PARENT_MANAGEDso that the mipi-i3c-hci core driver expects runtime PM to be controlled by the PCI parent rather than by individual instances. For all Intel HCI PCI configurations, enable the correspondingcontrol_instance_pmflag in the PCI driver.
Quoted 2 lines
Signed-off-by Adrian Hunter <[email protected]>---
Quoted 58 lines
Changes in V2:
Retain
HCI_QUIRK_RPM_ALLOWED
Amend commit message accordinglydrivers/i3c/master/mipi-i3c-hci/core.c | 4 +++- drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-)drivers/i3c/master/mipi-i3c-hci/core.c +3
-1@@ -1005,7 +1005,9 @@ static const struct acpi_device_id i3c_hci_acpi_match[] = {MODULE_DEVICE_TABLE (acpi, i3c_hci_acpi_match);static const struct platform_device_id i3c_hci_driver_ids[] = { - { .name ="intel-lpss-i3c" ,HCI_QUIRK_RPM_ALLOWED }, + { .name ="intel-lpss-i3c" ,HCI_QUIRK_RPM_ALLOWED | +HCI_QUIRK_RPM_IBI_ALLOWED | +HCI_QUIRK_RPM_PARENT_MANAGED }, {/* sentinel */ } };MODULE_DEVICE_TABLE (platform, i3c_hci_driver_ids);drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +3
-1@@ -200,6 +200,7 @@ static const struct mipi_i3c_hci_pci_info intel_mi_1_info = { .id = {0, 1}, .instance_offset = {0, 0x400}, .instance_count = 2, + .control_instance_pm = true, }; static const struct mipi_i3c_hci_pci_info intel_mi_2_info = {@@ -209,6 +210,7 @@ static const struct mipi_i3c_hci_pci_info intel_mi_2_info = { .id = {2, 3}, .instance_offset = {0, 0x400}, .instance_count = 2, + .control_instance_pm = true, }; static const struct mipi_i3c_hci_pci_info intel_si_2_info = {@@ -218,6 +220,7 @@ static const struct mipi_i3c_hci_pci_info intel_si_2_info = { .id = {2}, .instance_offset = {0}, .instance_count = 1, + .control_instance_pm = true, };static int mipi_i3c_hci_pci_find_instance (struct mipi_i3c_hci_pci *hci,struct device *dev) --2.51.0