Commit 5e1b35cfd680fe9d67bb2754393ff8e1e8ddf713

Authored by Paul Osmialowski
Committed by Greg Kroah-Hartman
1 parent f1764f870c

i2c: s3c2410: fix ABBA deadlock by keeping clock prepared

commit 34e81ad5f0b60007c95995eb7803da7e00c6c611 upstream.

This patch solves deadlock between clock prepare mutex and regmap mutex reported
by Tomasz Figa in [1] by implementing solution from [2]: "always leave the clock
of the i2c controller in a prepared state".

[1] https://lkml.org/lkml/2014/7/2/171
[2] https://lkml.org/lkml/2014/7/2/207

On each i2c transfer handled by s3c24xx_i2c_xfer(), clk_prepare_enable() was
called, which calls clk_prepare() then clk_enable(). clk_prepare() takes
prepare_lock mutex before proceeding. Note that i2c transfer functions are
invoked from many places in kernel, typically with some other additional lock
held.

It may happen that function on CPU1 (e.g. regmap_update_bits()) has taken a
mutex (i.e. regmap lock mutex) then it attempts i2c communication in order to
proceed (so it needs to obtain clock related prepare_lock mutex during transfer
preparation stage due to clk_prepare() call). At the same time other task on
CPU0 wants to operate on clock (e.g. to (un)prepare clock for some other reason)
so it has taken prepare_lock mutex.

CPU0:                        CPU1:
clk_disable_unused()         regulator_disable()
  clk_prepare_lock()           map->lock(map->lock_arg)
  regmap_read()                s3c24xx_i2c_xfer()
    map->lock(map->lock_arg)     clk_prepare_lock()

Implemented solution from [2] leaves i2c clock prepared. Preparation is done in
s3c24xx_i2c_probe() function. Without this patch, it is immediately unprepared
by clk_disable_unprepare() call. I've replaced this call with clk_disable() and
I've added clk_unprepare() call in s3c24xx_i2c_remove().

The s3c24xx_i2c_xfer() function now uses clk_enable() instead of
clk_prepare_enable() (and clk_disable() instead of clk_unprepare_disable()).

Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Showing 1 changed file with 17 additions and 6 deletions Side-by-side Diff

drivers/i2c/busses/i2c-s3c2410.c
... ... @@ -778,14 +778,16 @@
778 778 int ret;
779 779  
780 780 pm_runtime_get_sync(&adap->dev);
781   - clk_prepare_enable(i2c->clk);
  781 + ret = clk_enable(i2c->clk);
  782 + if (ret)
  783 + return ret;
782 784  
783 785 for (retry = 0; retry < adap->retries; retry++) {
784 786  
785 787 ret = s3c24xx_i2c_doxfer(i2c, msgs, num);
786 788  
787 789 if (ret != -EAGAIN) {
788   - clk_disable_unprepare(i2c->clk);
  790 + clk_disable(i2c->clk);
789 791 pm_runtime_put(&adap->dev);
790 792 return ret;
791 793 }
... ... @@ -795,7 +797,7 @@
795 797 udelay(100);
796 798 }
797 799  
798   - clk_disable_unprepare(i2c->clk);
  800 + clk_disable(i2c->clk);
799 801 pm_runtime_put(&adap->dev);
800 802 return -EREMOTEIO;
801 803 }
... ... @@ -1174,7 +1176,7 @@
1174 1176  
1175 1177 clk_prepare_enable(i2c->clk);
1176 1178 ret = s3c24xx_i2c_init(i2c);
1177   - clk_disable_unprepare(i2c->clk);
  1179 + clk_disable(i2c->clk);
1178 1180 if (ret != 0) {
1179 1181 dev_err(&pdev->dev, "I2C controller init failed\n");
1180 1182 return ret;
... ... @@ -1187,6 +1189,7 @@
1187 1189 i2c->irq = ret = platform_get_irq(pdev, 0);
1188 1190 if (ret <= 0) {
1189 1191 dev_err(&pdev->dev, "cannot find IRQ\n");
  1192 + clk_unprepare(i2c->clk);
1190 1193 return ret;
1191 1194 }
1192 1195  
... ... @@ -1195,6 +1198,7 @@
1195 1198  
1196 1199 if (ret != 0) {
1197 1200 dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
  1201 + clk_unprepare(i2c->clk);
1198 1202 return ret;
1199 1203 }
1200 1204 }
... ... @@ -1202,6 +1206,7 @@
1202 1206 ret = s3c24xx_i2c_register_cpufreq(i2c);
1203 1207 if (ret < 0) {
1204 1208 dev_err(&pdev->dev, "failed to register cpufreq notifier\n");
  1209 + clk_unprepare(i2c->clk);
1205 1210 return ret;
1206 1211 }
1207 1212  
... ... @@ -1218,6 +1223,7 @@
1218 1223 if (ret < 0) {
1219 1224 dev_err(&pdev->dev, "failed to add bus to i2c core\n");
1220 1225 s3c24xx_i2c_deregister_cpufreq(i2c);
  1226 + clk_unprepare(i2c->clk);
1221 1227 return ret;
1222 1228 }
1223 1229  
... ... @@ -1239,6 +1245,8 @@
1239 1245 {
1240 1246 struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
1241 1247  
  1248 + clk_unprepare(i2c->clk);
  1249 +
1242 1250 pm_runtime_disable(&i2c->adap.dev);
1243 1251 pm_runtime_disable(&pdev->dev);
1244 1252  
1245 1253  
1246 1254  
... ... @@ -1267,10 +1275,13 @@
1267 1275 {
1268 1276 struct platform_device *pdev = to_platform_device(dev);
1269 1277 struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
  1278 + int ret;
1270 1279  
1271   - clk_prepare_enable(i2c->clk);
  1280 + ret = clk_enable(i2c->clk);
  1281 + if (ret)
  1282 + return ret;
1272 1283 s3c24xx_i2c_init(i2c);
1273   - clk_disable_unprepare(i2c->clk);
  1284 + clk_disable(i2c->clk);
1274 1285 i2c->suspended = 0;
1275 1286  
1276 1287 return 0;