From: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxxxx>
Instead of repeatedly calling clk_get_rate for each transfer, register
a clock notifier to update the cached divider value each time the clock
rate actually changes.
A deadlock has been observed while adding tlv320aic32x4 audio codec to
the system. When this clock provider adds its clock, the clk mutex is
locked already, it needs to access i2c, which in return needs the mutex
for clk_get_rate as well.
Fixes: a55fa9d0e42e ("i2c: imx-lpi2c: add low power i2c bus driver")
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxxxx>
Reviewed-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
---
Changes in v8:
* Improved commit message describing an actual observed deadlock
Changes in v7:
* Use dev_err_probe
* Reworked/Shortened the commit message
It is not about saving CPU cycles, but to avoid locking the clk subsystem
upon each transfer.
drivers/i2c/busses/i2c-imx-lpi2c.c | 40 +++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 678b30e90492a..3070e605fd8ff 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -5,6 +5,7 @@
* Copyright 2016 Freescale Semiconductor, Inc.
*/
+#include <linux/atomic.h>
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/delay.h>
@@ -99,6 +100,8 @@ struct lpi2c_imx_struct {
__u8 *rx_buf;
__u8 *tx_buf;
struct completion complete;
+ struct notifier_block clk_change_nb;
+ atomic_t rate_per;
unsigned int msglen;
unsigned int delivered;
unsigned int block_data;
@@ -197,6 +200,20 @@ static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx)
} while (1);
}
+static int lpi2c_imx_clk_change_cb(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct clk_notifier_data *ndata = data;
+ struct lpi2c_imx_struct *lpi2c_imx = container_of(nb,
+ struct lpi2c_imx_struct,
+ clk_change_nb);
+
+ if (action & POST_RATE_CHANGE)
+ atomic_set(&lpi2c_imx->rate_per, ndata->new_rate);
+
+ return NOTIFY_OK;
+}
+
/* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */
static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx)
{
@@ -207,7 +224,7 @@ static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx)
lpi2c_imx_set_mode(lpi2c_imx);
- clk_rate = clk_get_rate(lpi2c_imx->clks[0].clk);
+ clk_rate = atomic_read(&lpi2c_imx->rate_per);
if (!clk_rate)
return -EINVAL;
@@ -590,6 +607,27 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
if (ret)
return ret;
+ lpi2c_imx->clk_change_nb.notifier_call = lpi2c_imx_clk_change_cb;
+ ret = devm_clk_notifier_register(&pdev->dev, lpi2c_imx->clks[0].clk,
+ &lpi2c_imx->clk_change_nb);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "can't register peripheral clock notifier\n");
+ /*
+ * Lock the clock rate to avoid rate change between clk_get_rate() and
+ * atomic_set()
+ */
+ ret = clk_rate_exclusive_get(lpi2c_imx->clks[0].clk);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "can't lock I2C peripheral clock rate\n");
+
+ atomic_set(&lpi2c_imx->rate_per, clk_get_rate(lpi2c_imx->clks[0].clk));
+ clk_rate_exclusive_put(lpi2c_imx->clks[0].clk);
+ if (!atomic_read(&lpi2c_imx->rate_per))
+ 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);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_get_noresume(&pdev->dev);
--
2.34.1
