| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20251125084718.2156168-1-carlos.song@nxp.com>
Date: Tue, 25 Nov 2025 16:47:18 +0800
From: Carlos Song <carlos.song@....com>
To: frank.li@....com,
aisheng.dong@....com,
andi.shyti@...nel.org,
shawnguo@...nel.org,
s.hauer@...gutronix.de,
kernel@...gutronix.de,
festevam@...il.com
Cc: linux-i2c@...r.kernel.org,
imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org,
Carlos Song <carlos.song@....com>
Subject: [PATCH v2] i2c: imx-lpi2c: Add runtime PM support for IRQ and clock management on i.MX8QXP/8QM
On i.MX8QXP/8QM SoCs, both the lvds/mipi and lvds/mipi-lpi2c power domains
must enter low-power mode during runtime suspend to achieve deep power
savings.
LPI2C resides in the lvds-lpi2c/mipi-lpi2c power domain, while its IRQ is
routed through an irqsteer located in the lvds/mipi power domain. The LPI2C
clock source comes from an LPCG within the lvds-lpi2c domain.
For example, the hierarchy for lvds0 and lvds0-lpi2c0 domains is:
┌───────────────────────┐
│ pm-domain : lvds0 │
│ │
│ ┌──────────────┐ │
│ │ irqsteer │ │
│ └───────▲──────┘ │
│ │irq │
│ │ │
└────────────┼──────────┘
┌────────────┼──────────┐
│ ┌───┼───┐ │
│ │lpi2c0 │ │
│ └───┬───┘clk │
│ ┌────────┼───────┐ │
│ │ LPCG │ │
│ └────────────────┘ │
│pm-domain:lvds0-lpi2c0 │
└───────────────────────┘
To allow these domains to power down in system runtime suspend:
- All irqsteer clients must release IRQs.
- All LPCG clients must disable and unprepare clocks.
Thus, LPI2C must:
- Free its IRQ during runtime suspend and re-request it on resume.
- Disable and unprepare all clocks during runtime suspend and prepare
and rne ble them on resume.
This enables the lvds/mipi domains to enter deep low-power mode,
significantly reducing power consumption compared to active mode.
Signed-off-by: Carlos Song <carlos.song@....com>
---
Changes since V1:
* Add unit for I2C_PM_LONG_TIMEOUT to I2C_PM_LONG_TIMEOUT_MS
* Reuqest lpi2c_imx->irq uncondtionally
* Remove help function and direct put code in original
lpi2c_runtime_suspend() and lpi2c_runtime_resume()
* Give more comments to explain why prolong PM timeout
---
1. Why not apply prepare and unprepare clocks management for all platforms:
As the report from me early:
https://lists.openwall.net/linux-kernel/2025/07/01/139
Scope of global prepare_lock is too big, it will cause dead clock
between RPM and prepare_lock in some specail case. But clock
prepare/unprepare is also necessary for low power consumption in I.MX8QXP
and 8QM, so I add separate clock management for these platforms to
avoid impacting other I.MX platforms. But it's possible that some
customers might encounter deadlock issues in IMX8QXP/8QM, so I prolong the
runtime PM timeout for 8QXP/QM platforms, which is currently a suitable
workaround method I think.
The dead lock happen as below call stacks
Task 117 Task 120
schedule()
clk_prepare_lock()--> wait prepare_lock(mutex_lock) schedule() wait for power.runtime_status exit RPM_SUSPENDING
^^^^ A ^^^^ B
clk_bulk_unprepare() rpm_resume()
lpi2c_runtime_suspend() pm_runtime_resume_and_get()
... lpi2c_imx_xfer()
...
rpm_suspend() set RPM_SUSPENDING pcf857x_set();
^^^^ B ...
clk_prepare_lock() --> hold prepare_lock
^^^^ A
...
Task 117 set power.runtime_status to RPM_SUSPENDING (A) and wait for task 120 release clock's global prepare mutex (B).
Task 120 hold global prepare mutex (B) and wait for power.runtime_status finish suspend (A).
So if RPM doesn't enter auto suspend too quick after hold prepare lock, this dead lock can be avoided.
So I prolong the runtime PM timeout, it can ensure that LPI2C does not enter auto suspend mode too
frequently. It has been verified valid for the above case.
2. Low power status report
Power domain status can be shown in pm_genpd_summary and sc firmware. Take
8QM platform lvds0 and lvds0-lpi2c0 power domain as example, before apply
this patch set:
root@...8qmmek:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
domain status children performance
/device runtime status managed by
---------------------------------------------------------------------------
lvds0-lpi2c0 on 0
lvds0_i2c0_clk active 0 SW
56243014.clock-controller active 0 SW
56247000.i2c suspended 0 SW
lvds0 on 0
lvds0_bypass_clk suspended 0 SW
lvds0_pixel_clk suspended 0 SW
lvds0_phy_clk suspended 0 SW
56240000.interrupt-controller active 0 SW
sc firmware will show the power domain status:
>$ power.r
LVDS_0 = on
LVDS_0_I2C_0 = on
After apply this patch set:
root@...8qmmek:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
domain status children performance
/device runtime status managed by
---------------------------------------------------------------------------
lvds0-lpi2c0 off-0 0
lvds0_i2c0_clk suspended 0 SW
56243014.clock-controller suspended 0 SW
56247000.i2c suspended 0 SW
lvds0 off-0 0
lvds0_bypass_clk suspended 0 SW
lvds0_pixel_clk suspended 0 SW
lvds0_phy_clk suspended 0 SW
56240000.interrupt-controller suspended 0 SW
sc firmware will show the power domain status:
>$ power.r
LVDS_0 = lp
LVDS_0_I2C_0 = lp
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 84 +++++++++++++++++++++++++-----
1 file changed, 71 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index dfacb0aec3c0..41ad82595583 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -132,6 +132,7 @@
#define CHUNK_DATA 256
#define I2C_PM_TIMEOUT 10 /* ms */
+#define I2C_PM_LONG_TIMEOUT_MS 1000 /* Avoid dead lock caused by big clock prepare lock */
#define I2C_DMA_THRESHOLD 8 /* bytes */
enum lpi2c_imx_mode {
@@ -149,6 +150,11 @@ enum lpi2c_imx_pincfg {
FOUR_PIN_PP,
};
+struct imx_lpi2c_hwdata {
+ bool need_request_free_irq; /* Needed by irqsteer */
+ bool need_prepare_unprepare_clk; /* Needed by LPCG */
+};
+
struct lpi2c_imx_dma {
bool using_pio_mode;
u8 rx_cmd_buf_len;
@@ -187,6 +193,21 @@ struct lpi2c_imx_struct {
bool can_use_dma;
struct lpi2c_imx_dma *dma;
struct i2c_client *target;
+ int irq;
+ const struct imx_lpi2c_hwdata *hwdata;
+};
+
+static const struct imx_lpi2c_hwdata imx7ulp_lpi2c_hwdata = {
+};
+
+static const struct imx_lpi2c_hwdata imx8qxp_lpi2c_hwdata = {
+ .need_request_free_irq = true,
+ .need_prepare_unprepare_clk = true,
+};
+
+static const struct imx_lpi2c_hwdata imx8qm_lpi2c_hwdata = {
+ .need_request_free_irq = true,
+ .need_prepare_unprepare_clk = true,
};
#define lpi2c_imx_read_msr_poll_timeout(atomic, val, cond) \
@@ -1424,7 +1445,9 @@ static const struct i2c_algorithm lpi2c_imx_algo = {
};
static const struct of_device_id lpi2c_imx_of_match[] = {
- { .compatible = "fsl,imx7ulp-lpi2c" },
+ { .compatible = "fsl,imx7ulp-lpi2c", .data = &imx7ulp_lpi2c_hwdata,},
+ { .compatible = "fsl,imx8qxp-lpi2c", .data = &imx8qxp_lpi2c_hwdata,},
+ { .compatible = "fsl,imx8qm-lpi2c", .data = &imx8qm_lpi2c_hwdata,},
{ }
};
MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match);
@@ -1435,19 +1458,23 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
struct resource *res;
dma_addr_t phy_addr;
unsigned int temp;
- int irq, ret;
+ int ret;
lpi2c_imx = devm_kzalloc(&pdev->dev, sizeof(*lpi2c_imx), GFP_KERNEL);
if (!lpi2c_imx)
return -ENOMEM;
+ lpi2c_imx->hwdata = of_device_get_match_data(&pdev->dev);
+ if (!lpi2c_imx->hwdata)
+ return -ENODEV;
+
lpi2c_imx->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
if (IS_ERR(lpi2c_imx->base))
return PTR_ERR(lpi2c_imx->base);
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
+ lpi2c_imx->irq = platform_get_irq(pdev, 0);
+ if (lpi2c_imx->irq < 0)
+ return lpi2c_imx->irq;
lpi2c_imx->adapter.owner = THIS_MODULE;
lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
@@ -1467,10 +1494,10 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
if (ret)
lpi2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
- ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
+ ret = devm_request_irq(&pdev->dev, lpi2c_imx->irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
pdev->name, lpi2c_imx);
if (ret)
- return dev_err_probe(&pdev->dev, ret, "can't claim irq %d\n", irq);
+ return dev_err_probe(&pdev->dev, ret, "can't claim irq %d\n", lpi2c_imx->irq);
i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
platform_set_drvdata(pdev, lpi2c_imx);
@@ -1493,7 +1520,11 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, -EINVAL,
"can't get I2C peripheral clock rate\n");
- pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
+ if (lpi2c_imx->hwdata->need_prepare_unprepare_clk)
+ pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_LONG_TIMEOUT_MS);
+ else
+ pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
+
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_get_noresume(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
@@ -1548,8 +1579,16 @@ static void lpi2c_imx_remove(struct platform_device *pdev)
static int __maybe_unused lpi2c_runtime_suspend(struct device *dev)
{
struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
+ bool need_prepare_unprepare_clk = lpi2c_imx->hwdata->need_prepare_unprepare_clk;
+ bool need_request_free_irq = lpi2c_imx->hwdata->need_request_free_irq;
+
+ if (need_request_free_irq)
+ devm_free_irq(dev, lpi2c_imx->irq, lpi2c_imx);
- clk_bulk_disable(lpi2c_imx->num_clks, lpi2c_imx->clks);
+ if (need_prepare_unprepare_clk)
+ clk_bulk_disable_unprepare(lpi2c_imx->num_clks, lpi2c_imx->clks);
+ else
+ clk_bulk_disable(lpi2c_imx->num_clks, lpi2c_imx->clks);
pinctrl_pm_select_sleep_state(dev);
return 0;
@@ -1558,13 +1597,32 @@ static int __maybe_unused lpi2c_runtime_suspend(struct device *dev)
static int __maybe_unused lpi2c_runtime_resume(struct device *dev)
{
struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
+ bool need_prepare_unprepare_clk = lpi2c_imx->hwdata->need_prepare_unprepare_clk;
+ bool need_request_free_irq = lpi2c_imx->hwdata->need_request_free_irq;
int ret;
pinctrl_pm_select_default_state(dev);
- ret = clk_bulk_enable(lpi2c_imx->num_clks, lpi2c_imx->clks);
- if (ret) {
- dev_err(dev, "failed to enable I2C clock, ret=%d\n", ret);
- return ret;
+ if (need_prepare_unprepare_clk) {
+ ret = clk_bulk_prepare_enable(lpi2c_imx->num_clks, lpi2c_imx->clks);
+ if (ret) {
+ dev_err(dev, "failed to enable I2C clock, ret=%d\n", ret);
+ return ret;
+ }
+ } else {
+ ret = clk_bulk_enable(lpi2c_imx->num_clks, lpi2c_imx->clks);
+ if (ret) {
+ dev_err(dev, "failed to enable clock %d\n", ret);
+ return ret;
+ }
+ }
+
+ if (need_request_free_irq) {
+ ret = devm_request_irq(dev, lpi2c_imx->irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
+ dev_name(dev), lpi2c_imx);
+ if (ret) {
+ dev_err(dev, "can't claim irq %d\n", lpi2c_imx->irq);
+ return ret;
+ }
}
return 0;
--
2.34.1
Powered by blists - more mailing lists