patch "w1: ds2490: USB transfer buffers need to be DMAable" added to char-misc-next

patch "w1: ds2490: USB transfer buffers need to be DMAable" added to char-misc-next

[gregkh]

This is a note to let you know that I've just added the patch titled

    w1: ds2490: USB transfer buffers need to be DMAable

to my char-misc git tree which can be found at
    git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
in the char-misc-next branch.

The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)

The patch will also be merged in the next major kernel release
during the merge window.

If you have any questions about this process, please let me know.


>From 61cd1b4cd1e8f7f7642ab64529d9bd52e8374641 Mon Sep 17 00:00:00 2001
From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Date: Wed, 18 Jan 2017 21:31:11 +0100
Subject: w1: ds2490: USB transfer buffers need to be DMAable

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 <stable@vger.kernel.org>
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.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)
-- 
2.11.0