Skip to content

Commit 8b4fc24

Browse files
AngeloGioacchino Del Regnowsakernel
authored andcommitted
i2c: mediatek: Optimize master_xfer() and avoid circular locking
Especially (but not only) during probe, it may happen that multiple devices are communicating via i2c (or multiple i2c busses) and sometimes while others are probing asynchronously. For example, a Cr50 TPM may be filling entropy (or userspace may be reading random data) while the rt5682 (i2c) codec driver reads/sets some registers, like while getting/setting a clock's rate, which happens both during probe and during system operation. In this driver, the mtk_i2c_transfer() function (which is the i2c .master_xfer() callback) was granularly managing the clocks by performing a clk_bulk_prepare_enable() to start them and its inverse. This is not only creating possible circular locking dependencies in the some cases (like former explanation), but it's also suboptimal, as clk_core prepare/unprepare operations are using mutex locking, which creates a bit of unwanted overhead (for example, i2c trackpads will call master_xfer() every few milliseconds!). With this commit, we avoid both the circular locking and additional overhead by changing how we handle the clocks in this driver: - Prepare the clocks during probe (and PM resume) - Enable/disable clocks in mtk_i2c_transfer() - Unprepare the clocks only for driver removal (and PM suspend) For the sake of providing a full explanation: during probe, the clocks are not only prepared but also enabled, as this is needed for some hardware initialization but, after that, we are disabling but not unpreparing them, leaving an expected state for the aforementioned clock handling strategy. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> Reviewed-by: Qii Wang <qii.wang@mediatek.com> Signed-off-by: Wolfram Sang <wsa@kernel.org>
1 parent be18ce1 commit 8b4fc24

1 file changed

Lines changed: 7 additions & 4 deletions

File tree

drivers/i2c/busses/i2c-mt65xx.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,7 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
11771177
int left_num = num;
11781178
struct mtk_i2c *i2c = i2c_get_adapdata(adap);
11791179

1180-
ret = clk_bulk_prepare_enable(I2C_MT65XX_CLK_MAX, i2c->clocks);
1180+
ret = clk_bulk_enable(I2C_MT65XX_CLK_MAX, i2c->clocks);
11811181
if (ret)
11821182
return ret;
11831183

@@ -1231,7 +1231,7 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
12311231
ret = num;
12321232

12331233
err_exit:
1234-
clk_bulk_disable_unprepare(I2C_MT65XX_CLK_MAX, i2c->clocks);
1234+
clk_bulk_disable(I2C_MT65XX_CLK_MAX, i2c->clocks);
12351235
return ret;
12361236
}
12371237

@@ -1412,7 +1412,7 @@ static int mtk_i2c_probe(struct platform_device *pdev)
14121412
return ret;
14131413
}
14141414
mtk_i2c_init_hw(i2c);
1415-
clk_bulk_disable_unprepare(I2C_MT65XX_CLK_MAX, i2c->clocks);
1415+
clk_bulk_disable(I2C_MT65XX_CLK_MAX, i2c->clocks);
14161416

14171417
ret = devm_request_irq(&pdev->dev, irq, mtk_i2c_irq,
14181418
IRQF_NO_SUSPEND | IRQF_TRIGGER_NONE,
@@ -1439,6 +1439,8 @@ static int mtk_i2c_remove(struct platform_device *pdev)
14391439

14401440
i2c_del_adapter(&i2c->adap);
14411441

1442+
clk_bulk_unprepare(I2C_MT65XX_CLK_MAX, i2c->clocks);
1443+
14421444
return 0;
14431445
}
14441446

@@ -1448,6 +1450,7 @@ static int mtk_i2c_suspend_noirq(struct device *dev)
14481450
struct mtk_i2c *i2c = dev_get_drvdata(dev);
14491451

14501452
i2c_mark_adapter_suspended(&i2c->adap);
1453+
clk_bulk_unprepare(I2C_MT65XX_CLK_MAX, i2c->clocks);
14511454

14521455
return 0;
14531456
}
@@ -1465,7 +1468,7 @@ static int mtk_i2c_resume_noirq(struct device *dev)
14651468

14661469
mtk_i2c_init_hw(i2c);
14671470

1468-
clk_bulk_disable_unprepare(I2C_MT65XX_CLK_MAX, i2c->clocks);
1471+
clk_bulk_disable(I2C_MT65XX_CLK_MAX, i2c->clocks);
14691472

14701473
i2c_mark_adapter_resumed(&i2c->adap);
14711474

0 commit comments

Comments
 (0)