0112-Fix-grabbing-lock-from-atomic-context-in-i2c-driver.patch 6.7 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219
  1. From 58ee33ad32d4a00735252718f8bac3f7592af6e7 Mon Sep 17 00:00:00 2001
  2. From: jeanleflambeur <catalin.vasile@gmail.com>
  3. Date: Sun, 1 Feb 2015 12:35:38 +0100
  4. Subject: [PATCH 112/114] Fix grabbing lock from atomic context in i2c driver
  5. 2 main changes:
  6. - check for timeouts in the bcm2708_bsc_setup function as indicated by this comment:
  7. /* poll for transfer start bit (should only take 1-20 polls) */
  8. This implies that the setup function can now fail so account for this everywhere it's called
  9. - Removed the clk_get_rate call from inside the setup function as it locks a mutex and that's not ok since we call it from under a spin lock.
  10. removed dead code and update comment
  11. fixed typo in comment
  12. ---
  13. drivers/i2c/busses/i2c-bcm2708.c | 90 +++++++++++++++++++++++++++++-----------
  14. 1 file changed, 65 insertions(+), 25 deletions(-)
  15. --- a/drivers/i2c/busses/i2c-bcm2708.c
  16. +++ b/drivers/i2c/busses/i2c-bcm2708.c
  17. @@ -68,6 +68,7 @@
  18. #define BSC_S_TA 0x00000001
  19. #define I2C_TIMEOUT_MS 150
  20. +#define I2C_WAIT_LOOP_COUNT 40
  21. #define DRV_NAME "bcm2708_i2c"
  22. @@ -86,6 +87,7 @@ struct bcm2708_i2c {
  23. void __iomem *base;
  24. int irq;
  25. struct clk *clk;
  26. + u32 cdiv;
  27. struct completion done;
  28. @@ -109,10 +111,10 @@ static void bcm2708_i2c_init_pinmode(int
  29. int pin;
  30. u32 *gpio = ioremap(GPIO_BASE, SZ_16K);
  31. - BUG_ON(id != 0 && id != 1);
  32. + BUG_ON(id != 0 && id != 1);
  33. /* BSC0 is on GPIO 0 & 1, BSC1 is on GPIO 2 & 3 */
  34. for (pin = id*2+0; pin <= id*2+1; pin++) {
  35. -printk("bcm2708_i2c_init_pinmode(%d,%d)\n", id, pin);
  36. + printk("bcm2708_i2c_init_pinmode(%d,%d)\n", id, pin);
  37. INP_GPIO(pin); /* set mode to GPIO input first */
  38. SET_GPIO_ALT(pin, 0); /* set mode to ALT 0 */
  39. }
  40. @@ -151,16 +153,16 @@ static inline void bcm2708_bsc_fifo_fill
  41. bcm2708_wr(bi, BSC_FIFO, bi->msg->buf[bi->pos++]);
  42. }
  43. -static inline void bcm2708_bsc_setup(struct bcm2708_i2c *bi)
  44. +static inline int bcm2708_bsc_setup(struct bcm2708_i2c *bi)
  45. {
  46. - unsigned long bus_hz;
  47. u32 cdiv, s;
  48. u32 c = BSC_C_I2CEN | BSC_C_INTD | BSC_C_ST | BSC_C_CLEAR_1;
  49. + int wait_loops = I2C_WAIT_LOOP_COUNT;
  50. - bus_hz = clk_get_rate(bi->clk);
  51. - cdiv = bus_hz / baudrate;
  52. - if (cdiv > 0xffff)
  53. - cdiv = 0xffff;
  54. + /* Can't call clk_get_rate as it locks a mutex and here we are spinlocked.
  55. + * Use the value that we cached in the probe.
  56. + */
  57. + cdiv = bi->cdiv;
  58. if (bi->msg->flags & I2C_M_RD)
  59. c |= BSC_C_INTR | BSC_C_READ;
  60. @@ -177,17 +179,25 @@ static inline void bcm2708_bsc_setup(str
  61. - Both messages to same slave address
  62. - Write message can fit inside FIFO (16 bytes or less) */
  63. if ( (bi->nmsgs > 1) &&
  64. - !(bi->msg[0].flags & I2C_M_RD) && (bi->msg[1].flags & I2C_M_RD) &&
  65. - (bi->msg[0].addr == bi->msg[1].addr) && (bi->msg[0].len <= 16)) {
  66. + !(bi->msg[0].flags & I2C_M_RD) && (bi->msg[1].flags & I2C_M_RD) &&
  67. + (bi->msg[0].addr == bi->msg[1].addr) && (bi->msg[0].len <= 16)) {
  68. /* Fill FIFO with entire write message (16 byte FIFO) */
  69. - while (bi->pos < bi->msg->len)
  70. + while (bi->pos < bi->msg->len) {
  71. bcm2708_wr(bi, BSC_FIFO, bi->msg->buf[bi->pos++]);
  72. + }
  73. /* Start write transfer (no interrupts, don't clear FIFO) */
  74. bcm2708_wr(bi, BSC_C, BSC_C_I2CEN | BSC_C_ST);
  75. +
  76. /* poll for transfer start bit (should only take 1-20 polls) */
  77. do {
  78. s = bcm2708_rd(bi, BSC_S);
  79. - } while (!(s & (BSC_S_TA | BSC_S_ERR | BSC_S_CLKT | BSC_S_DONE)));
  80. + } while (!(s & (BSC_S_TA | BSC_S_ERR | BSC_S_CLKT | BSC_S_DONE)) && --wait_loops >= 0);
  81. +
  82. + /* did we time out or some error occured? */
  83. + if (wait_loops < 0 || (s & (BSC_S_ERR | BSC_S_CLKT))) {
  84. + return -1;
  85. + }
  86. +
  87. /* Send next read message before the write transfer finishes. */
  88. bi->nmsgs--;
  89. bi->msg++;
  90. @@ -197,6 +207,8 @@ static inline void bcm2708_bsc_setup(str
  91. }
  92. }
  93. bcm2708_wr(bi, BSC_C, c);
  94. +
  95. + return 0;
  96. }
  97. static irqreturn_t bcm2708_i2c_interrupt(int irq, void *dev_id)
  98. @@ -204,13 +216,15 @@ static irqreturn_t bcm2708_i2c_interrupt
  99. struct bcm2708_i2c *bi = dev_id;
  100. bool handled = true;
  101. u32 s;
  102. + int ret;
  103. spin_lock(&bi->lock);
  104. /* we may see camera interrupts on the "other" I2C channel
  105. - Just return if we've not sent anything */
  106. - if (!bi->nmsgs || !bi->msg )
  107. + Just return if we've not sent anything */
  108. + if (!bi->nmsgs || !bi->msg) {
  109. goto early_exit;
  110. + }
  111. s = bcm2708_rd(bi, BSC_S);
  112. @@ -218,13 +232,16 @@ static irqreturn_t bcm2708_i2c_interrupt
  113. bcm2708_bsc_reset(bi);
  114. bi->error = true;
  115. + bi->msg = 0; /* to inform the that all work is done */
  116. + bi->nmsgs = 0;
  117. /* wake up our bh */
  118. complete(&bi->done);
  119. } else if (s & BSC_S_DONE) {
  120. bi->nmsgs--;
  121. - if (bi->msg->flags & I2C_M_RD)
  122. + if (bi->msg->flags & I2C_M_RD) {
  123. bcm2708_bsc_fifo_drain(bi);
  124. + }
  125. bcm2708_bsc_reset(bi);
  126. @@ -232,8 +249,19 @@ static irqreturn_t bcm2708_i2c_interrupt
  127. /* advance to next message */
  128. bi->msg++;
  129. bi->pos = 0;
  130. - bcm2708_bsc_setup(bi);
  131. + ret = bcm2708_bsc_setup(bi);
  132. + if (ret < 0) {
  133. + bcm2708_bsc_reset(bi);
  134. + bi->error = true;
  135. + bi->msg = 0; /* to inform the that all work is done */
  136. + bi->nmsgs = 0;
  137. + /* wake up our bh */
  138. + complete(&bi->done);
  139. + goto early_exit;
  140. + }
  141. } else {
  142. + bi->msg = 0; /* to inform the that all work is done */
  143. + bi->nmsgs = 0;
  144. /* wake up our bh */
  145. complete(&bi->done);
  146. }
  147. @@ -266,22 +294,33 @@ static int bcm2708_i2c_master_xfer(struc
  148. bi->nmsgs = num;
  149. bi->error = false;
  150. - bcm2708_bsc_setup(bi);
  151. + ret = bcm2708_bsc_setup(bi);
  152. - /* unlockig _after_ the setup to avoid races with the interrupt routine */
  153. spin_unlock_irqrestore(&bi->lock, flags);
  154. - ret = wait_for_completion_timeout(&bi->done,
  155. - msecs_to_jiffies(I2C_TIMEOUT_MS));
  156. + /* check the result of the setup */
  157. + if (ret < 0)
  158. + {
  159. + dev_err(&adap->dev, "transfer setup timed out\n");
  160. + goto error_timeout;
  161. + }
  162. +
  163. + ret = wait_for_completion_timeout(&bi->done, msecs_to_jiffies(I2C_TIMEOUT_MS));
  164. if (ret == 0) {
  165. dev_err(&adap->dev, "transfer timed out\n");
  166. - spin_lock_irqsave(&bi->lock, flags);
  167. - bcm2708_bsc_reset(bi);
  168. - spin_unlock_irqrestore(&bi->lock, flags);
  169. - return -ETIMEDOUT;
  170. + goto error_timeout;
  171. }
  172. - return bi->error ? -EIO : num;
  173. + ret = bi->error ? -EIO : num;
  174. + return ret;
  175. +
  176. +error_timeout:
  177. + spin_lock_irqsave(&bi->lock, flags);
  178. + bcm2708_bsc_reset(bi);
  179. + bi->msg = 0; /* to inform the interrupt handler that there's nothing else to be done */
  180. + bi->nmsgs = 0;
  181. + spin_unlock_irqrestore(&bi->lock, flags);
  182. + return -ETIMEDOUT;
  183. }
  184. static u32 bcm2708_i2c_functionality(struct i2c_adapter *adap)
  185. @@ -406,6 +445,7 @@ static int bcm2708_i2c_probe(struct plat
  186. cdiv = 0xffff;
  187. baudrate = bus_hz / cdiv;
  188. }
  189. + bi->cdiv = cdiv;
  190. dev_info(&pdev->dev, "BSC%d Controller at 0x%08lx (irq %d) (baudrate %d)\n",
  191. pdev->id, (unsigned long)regs->start, irq, baudrate);