summaryrefslogtreecommitdiffstats
path: root/w1-ds2490-USB-transfer-buffers-need-to-be-DMAable.patch
blob: 7e902b10014ff857343edcb417a7081afc0f4656 (plain)
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
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
From patchwork Wed Jan 18 20:31:11 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: w1: ds2490: USB transfer buffers need to be DMAable
From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
X-Patchwork-Id: 9524693
Message-Id: <5ba98814-d0b0-fbd4-d631-eda3472f4017@maciej.szmigiero.name>
To: Evgeniy Polyakov <zbr@ioremap.net>
Cc: linux-kernel <linux-kernel@vger.kernel.org>
Date: Wed, 18 Jan 2017 21:31:11 +0100

ds2490 driver was doing USB transfers from / to buffers on a stack.
This is not permitted and made the driver non-working with vmapped stacks.

Since all these transfers are done under the same bus_mutex lock we can
simply use shared buffers in a device private structure for two most common
of them.

While we are at it, let's also fix a comparison between int and size_t in
ds9490r_search() which made the driver spin in this function if state
register get requests were failing.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Cc: stable@vger.kernel.org
---
 drivers/w1/masters/ds2490.c | 142 ++++++++++++++++++++++++++------------------
 1 file changed, 84 insertions(+), 58 deletions(-)

diff --git a/drivers/w1/masters/ds2490.c b/drivers/w1/masters/ds2490.c
index 049a884a756f..59d74d1b47a8 100644
--- a/drivers/w1/masters/ds2490.c
+++ b/drivers/w1/masters/ds2490.c
@@ -153,6 +153,9 @@ struct ds_device
 	 */
 	u16			spu_bit;
 
+	u8			st_buf[ST_SIZE];
+	u8			byte_buf;
+
 	struct w1_bus_master	master;
 };
 
@@ -174,7 +177,6 @@ struct ds_status
 	u8			data_in_buffer_status;
 	u8			reserved1;
 	u8			reserved2;
-
 };
 
 static struct usb_device_id ds_id_table [] = {
@@ -244,28 +246,6 @@ static int ds_send_control(struct ds_device *dev, u16 value, u16 index)
 	return err;
 }
 
-static int ds_recv_status_nodump(struct ds_device *dev, struct ds_status *st,
-				 unsigned char *buf, int size)
-{
-	int count, err;
-
-	memset(st, 0, sizeof(*st));
-
-	count = 0;
-	err = usb_interrupt_msg(dev->udev, usb_rcvintpipe(dev->udev,
-		dev->ep[EP_STATUS]), buf, size, &count, 1000);
-	if (err < 0) {
-		pr_err("Failed to read 1-wire data from 0x%x: err=%d.\n",
-		       dev->ep[EP_STATUS], err);
-		return err;
-	}
-
-	if (count >= sizeof(*st))
-		memcpy(st, buf, sizeof(*st));
-
-	return count;
-}
-
 static inline void ds_print_msg(unsigned char *buf, unsigned char *str, int off)
 {
 	pr_info("%45s: %8x\n", str, buf[off]);
@@ -324,6 +304,35 @@ static void ds_dump_status(struct ds_device *dev, unsigned char *buf, int count)
 	}
 }
 
+static int ds_recv_status(struct ds_device *dev, struct ds_status *st,
+			  bool dump)
+{
+	int count, err;
+
+	if (st)
+		memset(st, 0, sizeof(*st));
+
+	count = 0;
+	err = usb_interrupt_msg(dev->udev,
+				usb_rcvintpipe(dev->udev,
+					       dev->ep[EP_STATUS]),
+				dev->st_buf, sizeof(dev->st_buf),
+				&count, 1000);
+	if (err < 0) {
+		pr_err("Failed to read 1-wire data from 0x%x: err=%d.\n",
+		       dev->ep[EP_STATUS], err);
+		return err;
+	}
+
+	if (dump)
+		ds_dump_status(dev, dev->st_buf, count);
+
+	if (st && count >= sizeof(*st))
+		memcpy(st, dev->st_buf, sizeof(*st));
+
+	return count;
+}
+
 static void ds_reset_device(struct ds_device *dev)
 {
 	ds_send_control_cmd(dev, CTL_RESET_DEVICE, 0);
@@ -344,7 +353,6 @@ static void ds_reset_device(struct ds_device *dev)
 static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size)
 {
 	int count, err;
-	struct ds_status st;
 
 	/* Careful on size.  If size is less than what is available in
 	 * the input buffer, the device fails the bulk transfer and
@@ -359,14 +367,9 @@ static int ds_recv_data(struct ds_device *dev, unsigned char *buf, int size)
 	err = usb_bulk_msg(dev->udev, usb_rcvbulkpipe(dev->udev, dev->ep[EP_DATA_IN]),
 				buf, size, &count, 1000);
 	if (err < 0) {
-		u8 buf[ST_SIZE];
-		int count;
-
 		pr_info("Clearing ep0x%x.\n", dev->ep[EP_DATA_IN]);
 		usb_clear_halt(dev->udev, usb_rcvbulkpipe(dev->udev, dev->ep[EP_DATA_IN]));
-
-		count = ds_recv_status_nodump(dev, &st, buf, sizeof(buf));
-		ds_dump_status(dev, buf, count);
+		ds_recv_status(dev, NULL, true);
 		return err;
 	}
 
@@ -404,7 +407,6 @@ int ds_stop_pulse(struct ds_device *dev, int limit)
 {
 	struct ds_status st;
 	int count = 0, err = 0;
-	u8 buf[ST_SIZE];
 
 	do {
 		err = ds_send_control(dev, CTL_HALT_EXE_IDLE, 0);
@@ -413,7 +415,7 @@ int ds_stop_pulse(struct ds_device *dev, int limit)
 		err = ds_send_control(dev, CTL_RESUME_EXE, 0);
 		if (err)
 			break;
-		err = ds_recv_status_nodump(dev, &st, buf, sizeof(buf));
+		err = ds_recv_status(dev, &st, false);
 		if (err)
 			break;
 
@@ -456,18 +458,17 @@ int ds_detect(struct ds_device *dev, struct ds_status *st)
 
 static int ds_wait_status(struct ds_device *dev, struct ds_status *st)
 {
-	u8 buf[ST_SIZE];
 	int err, count = 0;
 
 	do {
 		st->status = 0;
-		err = ds_recv_status_nodump(dev, st, buf, sizeof(buf));
+		err = ds_recv_status(dev, st, false);
 #if 0
 		if (err >= 0) {
 			int i;
 			printk("0x%x: count=%d, status: ", dev->ep[EP_STATUS], err);
 			for (i=0; i<err; ++i)
-				printk("%02x ", buf[i]);
+				printk("%02x ", dev->st_buf[i]);
 			printk("\n");
 		}
 #endif
@@ -485,7 +486,7 @@ static int ds_wait_status(struct ds_device *dev, struct ds_status *st)
 	 * can do something with it).
 	 */
 	if (err > 16 || count >= 100 || err < 0)
-		ds_dump_status(dev, buf, err);
+		ds_dump_status(dev, dev->st_buf, err);
 
 	/* Extended data isn't an error.  Well, a short is, but the dump
 	 * would have already told the user that and we can't do anything
@@ -608,7 +609,6 @@ static int ds_write_byte(struct ds_device *dev, u8 byte)
 {
 	int err;
 	struct ds_status st;
-	u8 rbyte;
 
 	err = ds_send_control(dev, COMM_BYTE_IO | COMM_IM | dev->spu_bit, byte);
 	if (err)
@@ -621,11 +621,11 @@ static int ds_write_byte(struct ds_device *dev, u8 byte)
 	if (err)
 		return err;
 
-	err = ds_recv_data(dev, &rbyte, sizeof(rbyte));
+	err = ds_recv_data(dev, &dev->byte_buf, 1);
 	if (err < 0)
 		return err;
 
-	return !(byte == rbyte);
+	return !(byte == dev->byte_buf);
 }
 
 static int ds_read_byte(struct ds_device *dev, u8 *byte)
@@ -712,7 +712,6 @@ static void ds9490r_search(void *data, struct w1_master *master,
 	int err;
 	u16 value, index;
 	struct ds_status st;
-	u8 st_buf[ST_SIZE];
 	int search_limit;
 	int found = 0;
 	int i;
@@ -724,7 +723,12 @@ static void ds9490r_search(void *data, struct w1_master *master,
 	/* FIFO 128 bytes, bulk packet size 64, read a multiple of the
 	 * packet size.
 	 */
-	u64 buf[2*64/8];
+	const size_t bufsize = 2 * 64;
+	u64 *buf;
+
+	buf = kmalloc(bufsize, GFP_KERNEL);
+	if (!buf)
+		return;
 
 	mutex_lock(&master->bus_mutex);
 
@@ -745,10 +749,9 @@ static void ds9490r_search(void *data, struct w1_master *master,
 	do {
 		schedule_timeout(jtime);
 
-		if (ds_recv_status_nodump(dev, &st, st_buf, sizeof(st_buf)) <
-			sizeof(st)) {
+		err = ds_recv_status(dev, &st, false);
+		if (err < 0 || err < sizeof(st))
 			break;
-		}
 
 		if (st.data_in_buffer_status) {
 			/* Bulk in can receive partial ids, but when it does
@@ -758,7 +761,7 @@ static void ds9490r_search(void *data, struct w1_master *master,
 			 * bulk without first checking if status says there
 			 * is data to read.
 			 */
-			err = ds_recv_data(dev, (u8 *)buf, sizeof(buf));
+			err = ds_recv_data(dev, (u8 *)buf, bufsize);
 			if (err < 0)
 				break;
 			for (i = 0; i < err/8; ++i) {
@@ -794,9 +797,14 @@ static void ds9490r_search(void *data, struct w1_master *master,
 	}
 search_out:
 	mutex_unlock(&master->bus_mutex);
+	kfree(buf);
 }
 
 #if 0
+/*
+ * FIXME: if this disabled code is ever used in the future all ds_send_data()
+ * calls must be changed to use a DMAable buffer.
+ */
 static int ds_match_access(struct ds_device *dev, u64 init)
 {
 	int err;
@@ -845,13 +853,12 @@ static int ds_set_path(struct ds_device *dev, u64 init)
 
 static u8 ds9490r_touch_bit(void *data, u8 bit)
 {
-	u8 ret;
 	struct ds_device *dev = data;
 
-	if (ds_touch_bit(dev, bit, &ret))
+	if (ds_touch_bit(dev, bit, &dev->byte_buf))
 		return 0;
 
-	return ret;
+	return dev->byte_buf;
 }
 
 #if 0
@@ -866,13 +873,12 @@ static u8 ds9490r_read_bit(void *data)
 {
 	struct ds_device *dev = data;
 	int err;
-	u8 bit = 0;
 
-	err = ds_touch_bit(dev, 1, &bit);
+	err = ds_touch_bit(dev, 1, &dev->byte_buf);
 	if (err)
 		return 0;
 
-	return bit & 1;
+	return dev->byte_buf & 1;
 }
 #endif
 
@@ -887,32 +893,52 @@ static u8 ds9490r_read_byte(void *data)
 {
 	struct ds_device *dev = data;
 	int err;
-	u8 byte = 0;
 
-	err = ds_read_byte(dev, &byte);
+	err = ds_read_byte(dev, &dev->byte_buf);
 	if (err)
 		return 0;
 
-	return byte;
+	return dev->byte_buf;
 }
 
 static void ds9490r_write_block(void *data, const u8 *buf, int len)
 {
 	struct ds_device *dev = data;
+	u8 *tbuf;
+
+	if (len <= 0)
+		return;
+
+	tbuf = kmalloc(len, GFP_KERNEL);
+	if (!tbuf)
+		return;
 
-	ds_write_block(dev, (u8 *)buf, len);
+	memcpy(tbuf, buf, len);
+	ds_write_block(dev, tbuf, len);
+
+	kfree(tbuf);
 }
 
 static u8 ds9490r_read_block(void *data, u8 *buf, int len)
 {
 	struct ds_device *dev = data;
 	int err;
+	u8 *tbuf;
 
-	err = ds_read_block(dev, buf, len);
-	if (err < 0)
+	if (len <= 0)
+		return 0;
+
+	tbuf = kmalloc(len, GFP_KERNEL);
+	if (!tbuf)
 		return 0;
 
-	return len;
+	err = ds_read_block(dev, tbuf, len);
+	if (err >= 0)
+		memcpy(buf, tbuf, len);
+
+	kfree(tbuf);
+
+	return err >= 0 ? len : 0;
 }
 
 static u8 ds9490r_reset(void *data)