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