From f1332df4548af1702700fca42ff48a466825cca0 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Mon, 2 Dec 2024 09:26:58 +0100 Subject: [PATCH 1/3] i2c: keba: drop check because i2c_unregister_device() is NULL safe No need to check the argument of i2c_unregister_device() because the function itself does it. Signed-off-by: Wolfram Sang Signed-off-by: Andi Shyti --- drivers/i2c/busses/i2c-keba.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-keba.c b/drivers/i2c/busses/i2c-keba.c index 759732a07ef0..7b9ed2592f5b 100644 --- a/drivers/i2c/busses/i2c-keba.c +++ b/drivers/i2c/busses/i2c-keba.c @@ -464,12 +464,8 @@ static void ki2c_unregister_devices(struct ki2c *ki2c) { int i; - for (i = 0; i < ki2c->client_size; i++) { - struct i2c_client *client = ki2c->client[i]; - - if (client) - i2c_unregister_device(client); - } + for (i = 0; i < ki2c->client_size; i++) + i2c_unregister_device(ki2c->client[i]); } static int ki2c_register_devices(struct ki2c *ki2c) From e7563c7355a8452fc9924b8f91fc94b69144431f Mon Sep 17 00:00:00 2001 From: Carlos Song Date: Mon, 25 Nov 2024 22:15:21 +0800 Subject: [PATCH 2/3] i2c: imx: fix divide by zero warning Add "i2c_clk_rate / 2" check to avoid "divide by zero warning". i2c_clk_rate may be zero if i2c clock is disabled. Signed-off-by: Carlos Song Signed-off-by: Clark Wang Signed-off-by: Haibo Chen Reviewed-by: Frank Li Signed-off-by: Andi Shyti --- drivers/i2c/busses/i2c-imx.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index f751d231ded8..1536e690a33b 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -622,8 +622,8 @@ static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx) return 0; } -static void i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx, - unsigned int i2c_clk_rate) +static int i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx, + unsigned int i2c_clk_rate) { struct imx_i2c_clk_pair *i2c_clk_div = i2c_imx->hwdata->clk_div; unsigned int div; @@ -638,7 +638,11 @@ static void i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx, /* Divider value calculation */ if (i2c_imx->cur_clk == i2c_clk_rate) - return; + return 0; + + /* Keep the denominator of the following program always NOT equal to 0. */ + if (!(i2c_clk_rate / 2)) + return -EINVAL; i2c_imx->cur_clk = i2c_clk_rate; @@ -669,6 +673,8 @@ static void i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx, dev_dbg(&i2c_imx->adapter.dev, "IFDR[IC]=0x%x, REAL DIV=%d\n", i2c_clk_div[i].val, i2c_clk_div[i].div); #endif + + return 0; } static int i2c_imx_clk_notifier_call(struct notifier_block *nb, @@ -678,11 +684,12 @@ static int i2c_imx_clk_notifier_call(struct notifier_block *nb, struct imx_i2c_struct *i2c_imx = container_of(nb, struct imx_i2c_struct, clk_change_nb); + int ret = 0; if (action & POST_RATE_CHANGE) - i2c_imx_set_clk(i2c_imx, ndata->new_rate); + ret = i2c_imx_set_clk(i2c_imx, ndata->new_rate); - return NOTIFY_OK; + return notifier_from_errno(ret); } static int i2c_imx_start(struct imx_i2c_struct *i2c_imx, bool atomic) @@ -1781,7 +1788,11 @@ static int i2c_imx_probe(struct platform_device *pdev) i2c_imx->bitrate = pdata->bitrate; i2c_imx->clk_change_nb.notifier_call = i2c_imx_clk_notifier_call; clk_notifier_register(i2c_imx->clk, &i2c_imx->clk_change_nb); - i2c_imx_set_clk(i2c_imx, clk_get_rate(i2c_imx->clk)); + ret = i2c_imx_set_clk(i2c_imx, clk_get_rate(i2c_imx->clk)); + if (ret < 0) { + dev_err(&pdev->dev, "can't get I2C clock\n"); + goto clk_notifier_unregister; + } i2c_imx_reset_regs(i2c_imx); From ee5da79b4f2a24a90417b0ae661971c0a12f2a48 Mon Sep 17 00:00:00 2001 From: Carlos Song Date: Mon, 25 Nov 2024 22:21:08 +0800 Subject: [PATCH 3/3] i2c: imx: make controller available until system suspend_noirq() and from resume_noirq() Put runtime PM to resume state between suspend() and suspend_noirq(), resume_noirq() and resume(), because some I2C devices need the controller on to perform communication during this period. The controller can't be woken up once runtime pm is disabled and in runtime autosuspended state. The problem can be easily reproduced on the I.MX8MQ platform: the PMIC needs to be used to enable regulator when the system resumes. When the PMIC uses the I2C controller, I2C runtime pm has not been enabled, so in i2c xfer(), pm_runtime_resume_and_get() will return an error, which causes data transfer to fail. Therefore, regulators cannot be enabled and system resume hangs. Here is resume error log: [ 53.888902] galcore 38000000.gpu3d: PM: calling genpd_resume_noirq @ 529, parent: platform [ 53.897203] i2c_imx_xfer, pm_runtime_resume_and_get is -13 [ 53.902713] imx-pgc imx-pgc-domain.5: failed to enable regulator: -EACCES [ 53.909518] galcore 38000000.gpu3d: PM: genpd_resume_noirq returned 0 after 12331 usecs [ 53.917545] mxc_hantro 38300000.vpu: PM: calling genpd_resume_noirq @ 529, parent: soc@0 [ 53.925659] i2c_imx_xfer, pm_runtime_resume_and_get is -13 [ 53.931157] imx-pgc imx-pgc-domain.6: failed to enable regulator: -EACCES I.MX8MQ system resume normally after applying the fix. Here is resume log: [ 71.068807] galcore 38000000.gpu3d: PM: calling genpd_resume_noirq @ 530, parent: platform [ 71.077103] i2c_imx_xfer, pm_runtime_resume_and_get is 0 [ 71.083578] galcore 38000000.gpu3d: PM: genpd_resume_noirq returned 0 after 6490 usecs [ 71.091526] mxc_hantro 38300000.vpu: PM: calling genpd_resume_noirq @ 530, parent: soc@0 [ 71.099638] i2c_imx_xfer, pm_runtime_resume_and_get is 0 [ 71.106091] mxc_hantro 38300000.vpu: PM: genpd_resume_noirq returned 0 after 6458 usecs Signed-off-by: Carlos Song Signed-off-by: Haibo Chen Reviewed-by: Frank Li Link: https://lore.kernel.org/r/20241125142108.1613016-1-carlos.song@nxp.com Signed-off-by: Andi Shyti --- drivers/i2c/busses/i2c-imx.c | 39 +++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 1536e690a33b..b6ed270dd04b 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -1768,7 +1768,8 @@ static int i2c_imx_probe(struct platform_device *pdev) goto rpm_disable; /* Request IRQ */ - ret = request_irq(irq, i2c_imx_isr, IRQF_SHARED, pdev->name, i2c_imx); + ret = request_irq(irq, i2c_imx_isr, IRQF_SHARED | IRQF_NO_SUSPEND, + pdev->name, i2c_imx); if (ret) { dev_err(&pdev->dev, "can't claim irq %d\n", irq); goto rpm_disable; @@ -1886,7 +1887,43 @@ static int i2c_imx_runtime_resume(struct device *dev) return ret; } +static int i2c_imx_suspend(struct device *dev) +{ + /* + * Some I2C devices may need the I2C controller to remain active + * during resume_noirq() or suspend_noirq(). If the controller is + * autosuspended, there is no way to wake it up once runtime PM is + * disabled (in suspend_late()). + * + * During system resume, the I2C controller will be available only + * after runtime PM is re-enabled (in resume_early()). However, this + * may be too late for some devices. + * + * Wake up the controller in the suspend() callback while runtime PM + * is still enabled. The I2C controller will remain available until + * the suspend_noirq() callback (pm_runtime_force_suspend()) is + * called. During resume, the I2C controller can be restored by the + * resume_noirq() callback (pm_runtime_force_resume()). + * + * Finally, the resume() callback re-enables autosuspend, ensuring + * the I2C controller remains available until the system enters + * suspend_noirq() and from resume_noirq(). + */ + return pm_runtime_resume_and_get(dev); +} + +static int i2c_imx_resume(struct device *dev) +{ + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + + return 0; +} + static const struct dev_pm_ops i2c_imx_pm_ops = { + NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, + pm_runtime_force_resume) + SYSTEM_SLEEP_PM_OPS(i2c_imx_suspend, i2c_imx_resume) RUNTIME_PM_OPS(i2c_imx_runtime_suspend, i2c_imx_runtime_resume, NULL) };