w1: ds2490: USB transfer buffers need to be DMAable
authorMaciej S. Szmigiero <mail@maciej.szmigiero.name>
Wed, 18 Jan 2017 20:31:11 +0000 (21:31 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 25 Jan 2017 10:46:42 +0000 (11:46 +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 <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

index 049a884a756f8d1d5474ae2bc56f4dddcb089c35..59d74d1b47a88d82c54ee843ee368c7120150c41 100644 (file)
@@ -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)