Re: [PATCH 16/17] i3c: mipi-i3c-hci: Add optional Runtime PM support

Frank Li
2025-12-19T17:17:04Z

On Fri, Dec 19, 2025 at 04:45:33PM +0200, Adrian Hunter wrote:
> Implement optional Runtime PM support for the MIPI I3C HCI driver.
> Introduce runtime suspend and resume callbacks to manage bus state and
> restore hardware configuration after resume. Optionally enable autosuspend
> with a default delay of 1 second, and add helper functions to control
> Runtime PM during probe and remove.
>
> Read quirks from i3c_hci_driver_ids[] and set new quirk
> HCI_QUIRK_RPM_ALLOWED for intel-lpss-i3c devices to enable runtime PM for
> them.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/i3c/master/mipi-i3c-hci/core.c | 96 +++++++++++++++++++++++++-
> drivers/i3c/master/mipi-i3c-hci/hci.h | 2 +
> 2 files changed, 96 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 3b0609cb7da7..9a8d34ae4a66 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/platform_data/mipi-i3c-hci.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>
> #include "hci.h"
> #include "ext_caps.h"
> @@ -176,6 +177,7 @@ void i3c_hci_sync_irq_inactive(struct i3c_hci *hci)
> int irq = platform_get_irq(pdev, 0);
>
> reg_write(INTR_SIGNAL_ENABLE, 0x0);
> + hci->irq_inactive = true;
> synchronize_irq(irq);
> }
>
> @@ -558,6 +560,14 @@ static irqreturn_t i3c_hci_irq_handler(int irq, void *dev_id)
> irqreturn_t result = IRQ_NONE;
> u32 val;
>
> + /*
> + * The IRQ can be shared, so the handler may be called when the IRQ is
> + * due to a different device. That could happen when runtime suspended,
> + * so exit immediately if IRQs are not expected for this device.
> + */
> + if (hci->irq_inactive)
> + return IRQ_NONE;
> +
> val = reg_read(INTR_STATUS);
> reg_write(INTR_STATUS, val);
> dev_dbg(&hci->master.dev, "INTR_STATUS %#x", val);
> @@ -613,6 +623,70 @@ static int i3c_hci_software_reset(struct i3c_hci *hci)
> return 0;
> }
>
> +static int i3c_hci_runtime_suspend(struct device *dev)
> +{
> + struct i3c_hci *hci = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = i3c_hci_bus_disable(hci);
> + if (ret)
> + return ret;
> +
> + hci->io->suspend(hci);
> +
> + return 0;
> +}
> +
> +static int i3c_hci_runtime_resume(struct device *dev)
> +{
> + struct i3c_hci *hci = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = i3c_hci_software_reset(hci);
> + if (ret)
> + return -EIO;
> +
> + reg_write(INTR_SIGNAL_ENABLE, 0x0);
> + reg_write(INTR_STATUS_ENABLE, GENMASK(31, 10));
> +
> + reg_clear(HC_CONTROL, HC_CONTROL_PIO_MODE);
> +
> + reg_write(MASTER_DEVICE_ADDR,
> + MASTER_DYNAMIC_ADDR(hci->master.this->info.dyn_addr) |
> + MASTER_DYNAMIC_ADDR_VALID);
> +

Is it possible move these register write to one of help function?

> + mipi_i3c_hci_dat_v1.restore(hci);
> +
> + hci->irq_inactive = false;
> +
> + hci->io->resume(hci);
> +
> + reg_set(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
> +
> + return 0;
> +}
> +
> +#define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
> +
> +static void i3c_hci_rpm_enable(struct device *dev)
> +{
> + struct i3c_hci *hci = dev_get_drvdata(dev);
> +
> + pm_runtime_set_autosuspend_delay(dev, DEFAULT_AUTOSUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);

use devm_pm_runtime_enable() can simple some code

> +
> + hci->master.rpm_allowed = true;
> +}
> +
> +static void i3c_hci_rpm_disable(struct device *dev)
> +{
> + pm_runtime_disable(dev);
> + pm_runtime_set_suspended(dev);
> + pm_runtime_dont_use_autosuspend(dev);
> +}
> +
> static int i3c_hci_init(struct i3c_hci *hci)
> {
> bool size_in_dwords, mode_selector;
> @@ -806,6 +880,8 @@ static int i3c_hci_probe(struct platform_device *pdev)
> hci->master.dev.init_name = dev_name(&pdev->dev);
>
> hci->quirks = (unsigned long)device_get_match_data(&pdev->dev);
> + if (!hci->quirks && platform_get_device_id(pdev))
> + hci->quirks = platform_get_device_id(pdev)->driver_data;
>
> ret = i3c_hci_init(hci);
> if (ret)
> @@ -817,12 +893,20 @@ static int i3c_hci_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + if (hci->quirks & HCI_QUIRK_RPM_ALLOWED)
> + i3c_hci_rpm_enable(&pdev->dev);
> +
> ret = i3c_master_register(&hci->master, &pdev->dev,
> &i3c_hci_ops, false);
> if (ret)
> - return ret;
> + goto err_rpm_disable;
>
> return 0;
> +
> +err_rpm_disable:
> + if (hci->quirks & HCI_QUIRK_RPM_ALLOWED)
> + i3c_hci_rpm_disable(&pdev->dev);
> + return ret;
> }
>
> static void i3c_hci_remove(struct platform_device *pdev)
> @@ -830,6 +914,9 @@ static void i3c_hci_remove(struct platform_device *pdev)
> struct i3c_hci *hci = platform_get_drvdata(pdev);
>
> i3c_master_unregister(&hci->master);
> +
> + if (hci->quirks & HCI_QUIRK_RPM_ALLOWED)
> + i3c_hci_rpm_disable(&pdev->dev);
> }
>
> static const __maybe_unused struct of_device_id i3c_hci_of_match[] = {
> @@ -845,11 +932,15 @@ 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" },
> + { .name = "intel-lpss-i3c", HCI_QUIRK_RPM_ALLOWED },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(platform, i3c_hci_driver_ids);
>
> +static const struct dev_pm_ops i3c_hci_pm_ops = {
> + SET_RUNTIME_PM_OPS(i3c_hci_runtime_suspend, i3c_hci_runtime_resume, NULL)

new macro RUNTIME_PM_OPS

Frank
> +};
> +
> static struct platform_driver i3c_hci_driver = {
> .probe = i3c_hci_probe,
> .remove = i3c_hci_remove,
> @@ -858,6 +949,7 @@ static struct platform_driver i3c_hci_driver = {
> .name = "mipi-i3c-hci",
> .of_match_table = of_match_ptr(i3c_hci_of_match),
> .acpi_match_table = i3c_hci_acpi_match,
> + .pm = pm_ptr(&i3c_hci_pm_ops),
> },
> };
> module_platform_driver(i3c_hci_driver);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 38f927685d3b..6339adf4b67a 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -51,6 +51,7 @@ struct i3c_hci {
> void *io_data;
> const struct hci_cmd_ops *cmd;
> atomic_t next_cmd_tid;
> + bool irq_inactive;
> u32 caps;
> unsigned int quirks;
> unsigned int DAT_entries;
> @@ -143,6 +144,7 @@ struct i3c_hci_dev_data {
> #define HCI_QUIRK_PIO_MODE BIT(2) /* Set PIO mode for AMD platforms */
> #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 */
>
> /* global functions */
> void mipi_i3c_hci_resume(struct i3c_hci *hci);
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c