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
|
From 58ee33ad32d4a00735252718f8bac3f7592af6e7 Mon Sep 17 00:00:00 2001
From: jeanleflambeur <catalin.vasile@gmail.com>
Date: Sun, 1 Feb 2015 12:35:38 +0100
Subject: [PATCH 112/114] Fix grabbing lock from atomic context in i2c driver
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
---
drivers/i2c/busses/i2c-bcm2708.c | 90 +++++++++++++++++++++++++++++-----------
1 file changed, 65 insertions(+), 25 deletions(-)
--- a/drivers/i2c/busses/i2c-bcm2708.c
+++ b/drivers/i2c/busses/i2c-bcm2708.c
@@ -68,6 +68,7 @@
#define BSC_S_TA 0x00000001
#define I2C_TIMEOUT_MS 150
+#define I2C_WAIT_LOOP_COUNT 40
#define DRV_NAME "bcm2708_i2c"
@@ -86,6 +87,7 @@ struct bcm2708_i2c {
void __iomem *base;
int irq;
struct clk *clk;
+ u32 cdiv;
struct completion done;
@@ -109,10 +111,10 @@ static void bcm2708_i2c_init_pinmode(int
int pin;
u32 *gpio = ioremap(GPIO_BASE, 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 */
}
@@ -151,16 +153,16 @@ static inline void bcm2708_bsc_fifo_fill
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;
@@ -177,17 +179,25 @@ static inline void bcm2708_bsc_setup(str
- 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++;
@@ -197,6 +207,8 @@ static inline void bcm2708_bsc_setup(str
}
}
bcm2708_wr(bi, BSC_C, c);
+
+ return 0;
}
static irqreturn_t bcm2708_i2c_interrupt(int irq, void *dev_id)
@@ -204,13 +216,15 @@ static irqreturn_t bcm2708_i2c_interrupt
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);
@@ -218,13 +232,16 @@ static irqreturn_t bcm2708_i2c_interrupt
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);
@@ -232,8 +249,19 @@ static irqreturn_t bcm2708_i2c_interrupt
/* 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);
}
@@ -266,22 +294,33 @@ static int bcm2708_i2c_master_xfer(struc
bi->nmsgs = num;
bi->error = false;
- bcm2708_bsc_setup(bi);
+ ret = bcm2708_bsc_setup(bi);
- /* unlockig _after_ the setup to avoid races with the interrupt routine */
spin_unlock_irqrestore(&bi->lock, flags);
- 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)
@@ -406,6 +445,7 @@ static int bcm2708_i2c_probe(struct plat
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);
|
